From a7f03c6aa2844ab4a7a04d63afd4a4970dd669ac Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 12:55:05 -0800 Subject: [PATCH 01/11] feat: allow disabling Workbench, Package Manager, and Chronicle without data loss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the enable/disable/teardown pattern from Connect (PR #93) to Workbench, Package Manager, and Chronicle. - Add spec.{workbench,packageManager,chronicle}.enabled and .teardown fields to the Site CRD - Add Suspended field to Workbench, PackageManager, and Chronicle CRDs - Site controller uses three-way branching: enabled → reconcile, disabled → suspend (preserve data), teardown → delete CR - Product controllers skip serving resources when Suspended=true - Network policies cleaned up when products are disabled - Fix Workbench cleanupDeployedService (was a no-op) - Fix Chronicle CleanupChronicle (was a TODO no-op) - Add 9 tests (3 per product: disable-never-enabled, suspend, teardown) Closes #95 --- api/core/v1beta1/chronicle_types.go | 5 + api/core/v1beta1/packagemanager_types.go | 5 + api/core/v1beta1/site_types.go | 44 +++ api/core/v1beta1/workbench_types.go | 5 + api/core/v1beta1/zz_generated.deepcopy.go | 45 +++ .../core/v1beta1/chroniclespec.go | 9 + .../core/v1beta1/internalchroniclespec.go | 18 ++ .../v1beta1/internalpackagemanagerspec.go | 18 ++ .../core/v1beta1/internalworkbenchspec.go | 18 ++ .../core/v1beta1/packagemanagerspec.go | 9 + .../core/v1beta1/workbenchspec.go | 9 + .../crd/bases/core.posit.team_chronicles.yaml | 5 + .../core.posit.team_packagemanagers.yaml | 5 + config/crd/bases/core.posit.team_sites.yaml | 44 +++ .../bases/core.posit.team_workbenches.yaml | 5 + docs/api-reference.md | 6 + docs/guides/packagemanager-configuration.md | 51 +++ docs/guides/product-team-site-management.md | 18 ++ docs/guides/workbench-configuration.md | 51 +++ .../controller/core/chronicle_controller.go | 64 +++- internal/controller/core/package_manager.go | 31 ++ internal/controller/core/site_controller.go | 145 ++++++--- .../core/site_controller_chronicle.go | 61 ++++ .../core/site_controller_networkpolicies.go | 73 ++++- .../core/site_controller_package_manager.go | 61 ++++ .../core/site_controller_workbench.go | 61 ++++ internal/controller/core/site_test.go | 292 ++++++++++++++++++ internal/controller/core/workbench.go | 190 +++++++++++- 28 files changed, 1288 insertions(+), 60 deletions(-) diff --git a/api/core/v1beta1/chronicle_types.go b/api/core/v1beta1/chronicle_types.go index 14b137f..cb21c39 100644 --- a/api/core/v1beta1/chronicle_types.go +++ b/api/core/v1beta1/chronicle_types.go @@ -13,6 +13,11 @@ import ( // ChronicleSpec defines the desired state of Chronicle type ChronicleSpec struct { + // Suspended indicates Chronicle should not run serving resources (StatefulSet, Service) + // but should preserve configuration. Set by the Site controller. + // +optional + Suspended *bool `json:"suspended,omitempty"` + Config ChronicleConfig `json:"config,omitempty"` // ImagePullSecrets is a set of image pull secrets to use for all image pulls. These names / secrets diff --git a/api/core/v1beta1/packagemanager_types.go b/api/core/v1beta1/packagemanager_types.go index 0163572..1656d29 100644 --- a/api/core/v1beta1/packagemanager_types.go +++ b/api/core/v1beta1/packagemanager_types.go @@ -16,6 +16,11 @@ import ( // PackageManagerSpec defines the desired state of PackageManager type PackageManagerSpec struct { + // Suspended indicates Package Manager should not run serving resources (Deployment, Service, Ingress) + // but should preserve data resources (PVC, database, secrets). Set by the Site controller. + // +optional + Suspended *bool `json:"suspended,omitempty"` + License product.LicenseSpec `json:"license,omitempty"` Config *PackageManagerConfig `json:"config,omitempty"` Volume *product.VolumeSpec `json:"volume,omitempty"` diff --git a/api/core/v1beta1/site_types.go b/api/core/v1beta1/site_types.go index 44bb426..c1d1dd9 100644 --- a/api/core/v1beta1/site_types.go +++ b/api/core/v1beta1/site_types.go @@ -189,6 +189,21 @@ type FeatureEnablerConfig struct { } type InternalPackageManagerSpec struct { + // Enabled controls whether Package Manager is running. Defaults to true. + // Setting to false suspends Package Manager: stops pods and removes ingress/service, + // but preserves PVC, database, and secrets so data is retained. + // Re-enabling restores full service without data loss. + // +kubebuilder:default=true + // +optional + Enabled *bool `json:"enabled,omitempty"` + + // Teardown permanently destroys all Package Manager resources including the database, + // secrets, and persistent volume claim. Only takes effect when Enabled is false. + // Re-enabling after teardown starts fresh with a new empty database. + // +kubebuilder:default=false + // +optional + Teardown *bool `json:"teardown,omitempty"` + License product.LicenseSpec `json:"license,omitempty"` Volume *product.VolumeSpec `json:"volume,omitempty"` @@ -341,6 +356,21 @@ type InternalConnectExperimentalFeatures struct { } type InternalWorkbenchSpec struct { + // Enabled controls whether Workbench is running. Defaults to true. + // Setting to false suspends Workbench: stops pods and removes ingress/service, + // but preserves PVC, database, and secrets so data is retained. + // Re-enabling restores full service without data loss. + // +kubebuilder:default=true + // +optional + Enabled *bool `json:"enabled,omitempty"` + + // Teardown permanently destroys all Workbench resources including the database, + // secrets, and persistent volume claim. Only takes effect when Enabled is false. + // Re-enabling after teardown starts fresh with a new empty database. + // +kubebuilder:default=false + // +optional + Teardown *bool `json:"teardown,omitempty"` + Databricks map[string]DatabricksConfig `json:"databricks,omitempty"` Snowflake SnowflakeConfig `json:"snowflake,omitempty"` @@ -509,6 +539,20 @@ type InternalWorkbenchExperimentalFeatures struct { } type InternalChronicleSpec struct { + // Enabled controls whether Chronicle is running. Defaults to true. + // Setting to false suspends Chronicle: stops the StatefulSet and removes the service. + // Re-enabling restores full service. + // +kubebuilder:default=true + // +optional + Enabled *bool `json:"enabled,omitempty"` + + // Teardown permanently destroys all Chronicle resources. + // Only takes effect when Enabled is false. + // Re-enabling after teardown starts fresh. + // +kubebuilder:default=false + // +optional + Teardown *bool `json:"teardown,omitempty"` + NodeSelector map[string]string `json:"nodeSelector,omitempty"` Image string `json:"image,omitempty"` diff --git a/api/core/v1beta1/workbench_types.go b/api/core/v1beta1/workbench_types.go index 00cf0db..c9bc28f 100644 --- a/api/core/v1beta1/workbench_types.go +++ b/api/core/v1beta1/workbench_types.go @@ -25,6 +25,11 @@ const MaxLoginPageHtmlSize = 64 * 1024 // WorkbenchSpec defines the desired state of Workbench type WorkbenchSpec struct { + // Suspended indicates Workbench should not run serving resources (Deployment, Service, Ingress) + // but should preserve data resources (PVC, database, secrets). Set by the Site controller. + // +optional + Suspended *bool `json:"suspended,omitempty"` + License product.LicenseSpec `json:"license,omitempty"` Config WorkbenchConfig `json:"config,omitempty"` SecretConfig WorkbenchSecretConfig `json:"secretConfig,omitempty"` diff --git a/api/core/v1beta1/zz_generated.deepcopy.go b/api/core/v1beta1/zz_generated.deepcopy.go index dc0775b..94ddca2 100644 --- a/api/core/v1beta1/zz_generated.deepcopy.go +++ b/api/core/v1beta1/zz_generated.deepcopy.go @@ -321,6 +321,11 @@ func (in *ChronicleS3StorageConfig) DeepCopy() *ChronicleS3StorageConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ChronicleSpec) DeepCopyInto(out *ChronicleSpec) { *out = *in + if in.Suspended != nil { + in, out := &in.Suspended, &out.Suspended + *out = new(bool) + **out = **in + } in.Config.DeepCopyInto(&out.Config) if in.ImagePullSecrets != nil { in, out := &in.ImagePullSecrets, &out.ImagePullSecrets @@ -1100,6 +1105,16 @@ func (in *GPUSettings) DeepCopy() *GPUSettings { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InternalChronicleSpec) DeepCopyInto(out *InternalChronicleSpec) { *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } + if in.Teardown != nil { + in, out := &in.Teardown, &out.Teardown + *out = new(bool) + **out = **in + } if in.NodeSelector != nil { in, out := &in.NodeSelector, &out.NodeSelector *out = make(map[string]string, len(*in)) @@ -1270,6 +1285,16 @@ func (in *InternalKeycloakSpec) DeepCopy() *InternalKeycloakSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InternalPackageManagerSpec) DeepCopyInto(out *InternalPackageManagerSpec) { *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } + if in.Teardown != nil { + in, out := &in.Teardown, &out.Teardown + *out = new(bool) + **out = **in + } out.License = in.License if in.Volume != nil { in, out := &in.Volume, &out.Volume @@ -1365,6 +1390,16 @@ func (in *InternalWorkbenchExperimentalFeatures) DeepCopy() *InternalWorkbenchEx // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InternalWorkbenchSpec) DeepCopyInto(out *InternalWorkbenchSpec) { *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } + if in.Teardown != nil { + in, out := &in.Teardown, &out.Teardown + *out = new(bool) + **out = **in + } if in.Databricks != nil { in, out := &in.Databricks, &out.Databricks *out = make(map[string]DatabricksConfig, len(*in)) @@ -1802,6 +1837,11 @@ func (in *PackageManagerServerConfig) DeepCopy() *PackageManagerServerConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PackageManagerSpec) DeepCopyInto(out *PackageManagerSpec) { *out = *in + if in.Suspended != nil { + in, out := &in.Suspended, &out.Suspended + *out = new(bool) + **out = **in + } out.License = in.License if in.Config != nil { in, out := &in.Config, &out.Config @@ -3137,6 +3177,11 @@ func (in *WorkbenchSessionNewlineConfig) DeepCopy() *WorkbenchSessionNewlineConf // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WorkbenchSpec) DeepCopyInto(out *WorkbenchSpec) { *out = *in + if in.Suspended != nil { + in, out := &in.Suspended, &out.Suspended + *out = new(bool) + **out = **in + } out.License = in.License in.Config.DeepCopyInto(&out.Config) in.SecretConfig.DeepCopyInto(&out.SecretConfig) diff --git a/client-go/applyconfiguration/core/v1beta1/chroniclespec.go b/client-go/applyconfiguration/core/v1beta1/chroniclespec.go index 7848053..91794fe 100644 --- a/client-go/applyconfiguration/core/v1beta1/chroniclespec.go +++ b/client-go/applyconfiguration/core/v1beta1/chroniclespec.go @@ -8,6 +8,7 @@ package v1beta1 // ChronicleSpecApplyConfiguration represents a declarative configuration of the ChronicleSpec type for use // with apply. type ChronicleSpecApplyConfiguration struct { + Suspended *bool `json:"suspended,omitempty"` Config *ChronicleConfigApplyConfiguration `json:"config,omitempty"` ImagePullSecrets []string `json:"imagePullSecrets,omitempty"` NodeSelector map[string]string `json:"nodeSelector,omitempty"` @@ -24,6 +25,14 @@ func ChronicleSpec() *ChronicleSpecApplyConfiguration { return &ChronicleSpecApplyConfiguration{} } +// WithSuspended sets the Suspended field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Suspended field is set to the value of the last call. +func (b *ChronicleSpecApplyConfiguration) WithSuspended(value bool) *ChronicleSpecApplyConfiguration { + b.Suspended = &value + return b +} + // WithConfig sets the Config field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Config field is set to the value of the last call. diff --git a/client-go/applyconfiguration/core/v1beta1/internalchroniclespec.go b/client-go/applyconfiguration/core/v1beta1/internalchroniclespec.go index 8a46e39..380c3b9 100644 --- a/client-go/applyconfiguration/core/v1beta1/internalchroniclespec.go +++ b/client-go/applyconfiguration/core/v1beta1/internalchroniclespec.go @@ -12,6 +12,8 @@ import ( // InternalChronicleSpecApplyConfiguration represents a declarative configuration of the InternalChronicleSpec type for use // with apply. type InternalChronicleSpecApplyConfiguration struct { + Enabled *bool `json:"enabled,omitempty"` + Teardown *bool `json:"teardown,omitempty"` NodeSelector map[string]string `json:"nodeSelector,omitempty"` Image *string `json:"image,omitempty"` AddEnv map[string]string `json:"addEnv,omitempty"` @@ -26,6 +28,22 @@ func InternalChronicleSpec() *InternalChronicleSpecApplyConfiguration { return &InternalChronicleSpecApplyConfiguration{} } +// WithEnabled sets the Enabled field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Enabled field is set to the value of the last call. +func (b *InternalChronicleSpecApplyConfiguration) WithEnabled(value bool) *InternalChronicleSpecApplyConfiguration { + b.Enabled = &value + return b +} + +// WithTeardown sets the Teardown field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Teardown field is set to the value of the last call. +func (b *InternalChronicleSpecApplyConfiguration) WithTeardown(value bool) *InternalChronicleSpecApplyConfiguration { + b.Teardown = &value + return b +} + // WithNodeSelector puts the entries into the NodeSelector field in the declarative configuration // and returns the receiver, so that objects can be build by chaining "With" function invocations. // If called multiple times, the entries provided by each call will be put on the NodeSelector field, diff --git a/client-go/applyconfiguration/core/v1beta1/internalpackagemanagerspec.go b/client-go/applyconfiguration/core/v1beta1/internalpackagemanagerspec.go index c9e1297..62f59d0 100644 --- a/client-go/applyconfiguration/core/v1beta1/internalpackagemanagerspec.go +++ b/client-go/applyconfiguration/core/v1beta1/internalpackagemanagerspec.go @@ -13,6 +13,8 @@ import ( // InternalPackageManagerSpecApplyConfiguration represents a declarative configuration of the InternalPackageManagerSpec type for use // with apply. type InternalPackageManagerSpecApplyConfiguration struct { + Enabled *bool `json:"enabled,omitempty"` + Teardown *bool `json:"teardown,omitempty"` License *product.LicenseSpec `json:"license,omitempty"` Volume *product.VolumeSpec `json:"volume,omitempty"` NodeSelector map[string]string `json:"nodeSelector,omitempty"` @@ -34,6 +36,22 @@ func InternalPackageManagerSpec() *InternalPackageManagerSpecApplyConfiguration return &InternalPackageManagerSpecApplyConfiguration{} } +// WithEnabled sets the Enabled field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Enabled field is set to the value of the last call. +func (b *InternalPackageManagerSpecApplyConfiguration) WithEnabled(value bool) *InternalPackageManagerSpecApplyConfiguration { + b.Enabled = &value + return b +} + +// WithTeardown sets the Teardown field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Teardown field is set to the value of the last call. +func (b *InternalPackageManagerSpecApplyConfiguration) WithTeardown(value bool) *InternalPackageManagerSpecApplyConfiguration { + b.Teardown = &value + return b +} + // WithLicense sets the License field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the License field is set to the value of the last call. diff --git a/client-go/applyconfiguration/core/v1beta1/internalworkbenchspec.go b/client-go/applyconfiguration/core/v1beta1/internalworkbenchspec.go index 1776cfc..993434a 100644 --- a/client-go/applyconfiguration/core/v1beta1/internalworkbenchspec.go +++ b/client-go/applyconfiguration/core/v1beta1/internalworkbenchspec.go @@ -14,6 +14,8 @@ import ( // InternalWorkbenchSpecApplyConfiguration represents a declarative configuration of the InternalWorkbenchSpec type for use // with apply. type InternalWorkbenchSpecApplyConfiguration struct { + Enabled *bool `json:"enabled,omitempty"` + Teardown *bool `json:"teardown,omitempty"` Databricks map[string]DatabricksConfigApplyConfiguration `json:"databricks,omitempty"` Snowflake *SnowflakeConfigApplyConfiguration `json:"snowflake,omitempty"` License *product.LicenseSpec `json:"license,omitempty"` @@ -55,6 +57,22 @@ func InternalWorkbenchSpec() *InternalWorkbenchSpecApplyConfiguration { return &InternalWorkbenchSpecApplyConfiguration{} } +// WithEnabled sets the Enabled field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Enabled field is set to the value of the last call. +func (b *InternalWorkbenchSpecApplyConfiguration) WithEnabled(value bool) *InternalWorkbenchSpecApplyConfiguration { + b.Enabled = &value + return b +} + +// WithTeardown sets the Teardown field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Teardown field is set to the value of the last call. +func (b *InternalWorkbenchSpecApplyConfiguration) WithTeardown(value bool) *InternalWorkbenchSpecApplyConfiguration { + b.Teardown = &value + return b +} + // WithDatabricks puts the entries into the Databricks field in the declarative configuration // and returns the receiver, so that objects can be build by chaining "With" function invocations. // If called multiple times, the entries provided by each call will be put on the Databricks field, diff --git a/client-go/applyconfiguration/core/v1beta1/packagemanagerspec.go b/client-go/applyconfiguration/core/v1beta1/packagemanagerspec.go index 742eb42..0cc57b7 100644 --- a/client-go/applyconfiguration/core/v1beta1/packagemanagerspec.go +++ b/client-go/applyconfiguration/core/v1beta1/packagemanagerspec.go @@ -13,6 +13,7 @@ import ( // PackageManagerSpecApplyConfiguration represents a declarative configuration of the PackageManagerSpec type for use // with apply. type PackageManagerSpecApplyConfiguration struct { + Suspended *bool `json:"suspended,omitempty"` License *product.LicenseSpec `json:"license,omitempty"` Config *PackageManagerConfigApplyConfiguration `json:"config,omitempty"` Volume *product.VolumeSpec `json:"volume,omitempty"` @@ -45,6 +46,14 @@ func PackageManagerSpec() *PackageManagerSpecApplyConfiguration { return &PackageManagerSpecApplyConfiguration{} } +// WithSuspended sets the Suspended field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Suspended field is set to the value of the last call. +func (b *PackageManagerSpecApplyConfiguration) WithSuspended(value bool) *PackageManagerSpecApplyConfiguration { + b.Suspended = &value + return b +} + // WithLicense sets the License field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the License field is set to the value of the last call. diff --git a/client-go/applyconfiguration/core/v1beta1/workbenchspec.go b/client-go/applyconfiguration/core/v1beta1/workbenchspec.go index e7ce73e..a785ea9 100644 --- a/client-go/applyconfiguration/core/v1beta1/workbenchspec.go +++ b/client-go/applyconfiguration/core/v1beta1/workbenchspec.go @@ -13,6 +13,7 @@ import ( // WorkbenchSpecApplyConfiguration represents a declarative configuration of the WorkbenchSpec type for use // with apply. type WorkbenchSpecApplyConfiguration struct { + Suspended *bool `json:"suspended,omitempty"` License *product.LicenseSpec `json:"license,omitempty"` Config *WorkbenchConfigApplyConfiguration `json:"config,omitempty"` SecretConfig *WorkbenchSecretConfigApplyConfiguration `json:"secretConfig,omitempty"` @@ -55,6 +56,14 @@ func WorkbenchSpec() *WorkbenchSpecApplyConfiguration { return &WorkbenchSpecApplyConfiguration{} } +// WithSuspended sets the Suspended field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Suspended field is set to the value of the last call. +func (b *WorkbenchSpecApplyConfiguration) WithSuspended(value bool) *WorkbenchSpecApplyConfiguration { + b.Suspended = &value + return b +} + // WithLicense sets the License field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the License field is set to the value of the last call. diff --git a/config/crd/bases/core.posit.team_chronicles.yaml b/config/crd/bases/core.posit.team_chronicles.yaml index 12fd267..1032770 100644 --- a/config/crd/bases/core.posit.team_chronicles.yaml +++ b/config/crd/bases/core.posit.team_chronicles.yaml @@ -118,6 +118,11 @@ spec: additionalProperties: type: string type: object + suspended: + description: |- + Suspended indicates Chronicle should not run serving resources (StatefulSet, Service) + but should preserve configuration. Set by the Site controller. + type: boolean workloadCompoundName: description: WorkloadCompoundName is the name for the workload type: string diff --git a/config/crd/bases/core.posit.team_packagemanagers.yaml b/config/crd/bases/core.posit.team_packagemanagers.yaml index 72619fa..942a69c 100644 --- a/config/crd/bases/core.posit.team_packagemanagers.yaml +++ b/config/crd/bases/core.posit.team_packagemanagers.yaml @@ -321,6 +321,11 @@ spec: Sleep puts the service to sleep... so you can debug a crash looping container / etc. It is an ugly escape hatch, but can also be useful on occasion type: boolean + suspended: + description: |- + Suspended indicates Package Manager should not run serving resources (Deployment, Service, Ingress) + but should preserve data resources (PVC, database, secrets). Set by the Site controller. + type: boolean url: type: string volume: diff --git a/config/crd/bases/core.posit.team_sites.yaml b/config/crd/bases/core.posit.team_sites.yaml index 647b764..a240586 100644 --- a/config/crd/bases/core.posit.team_sites.yaml +++ b/config/crd/bases/core.posit.team_sites.yaml @@ -52,6 +52,13 @@ spec: type: object agentImage: type: string + enabled: + default: true + description: |- + Enabled controls whether Chronicle is running. Defaults to true. + Setting to false suspends Chronicle: stops the StatefulSet and removes the service. + Re-enabling restores full service. + type: boolean image: type: string imagePullPolicy: @@ -64,6 +71,13 @@ spec: type: object s3Bucket: type: string + teardown: + default: false + description: |- + Teardown permanently destroys all Chronicle resources. + Only takes effect when Enabled is false. + Re-enabling after teardown starts fresh. + type: boolean type: object clusterDate: description: ClusterDate is the date id (YYYYmmdd) for the cluster. @@ -654,6 +668,14 @@ spec: domainPrefix: default: packagemanager type: string + enabled: + default: true + description: |- + Enabled controls whether Package Manager is running. Defaults to true. + Setting to false suspends Package Manager: stops pods and removes ingress/service, + but preserves PVC, database, and secrets so data is retained. + Re-enabling restores full service without data loss. + type: boolean gitSSHKeys: description: |- GitSSHKeys defines SSH key configurations for Git authentication in Package Manager @@ -766,6 +788,13 @@ spec: type: integer s3Bucket: type: string + teardown: + default: false + description: |- + Teardown permanently destroys all Package Manager resources including the database, + secrets, and persistent volume claim. Only takes effect when Enabled is false. + Re-enabling after teardown starts fresh with a new empty database. + type: boolean volume: description: VolumeSpec is a specification for a PersistentVolumeClaim to be created (and/or mounted) @@ -1081,6 +1110,14 @@ spec: domainPrefix: default: workbench type: string + enabled: + default: true + description: |- + Enabled controls whether Workbench is running. Defaults to true. + Setting to false suspends Workbench: stops pods and removes ingress/service, + but preserves PVC, database, and secrets so data is retained. + Re-enabling restores full service without data loss. + type: boolean experimentalFeatures: description: ExperimentalFeatures allows enabling miscellaneous experimental features for workbench @@ -1518,6 +1555,13 @@ spec: clientId: type: string type: object + teardown: + default: false + description: |- + Teardown permanently destroys all Workbench resources including the database, + secrets, and persistent volume claim. Only takes effect when Enabled is false. + Re-enabling after teardown starts fresh with a new empty database. + type: boolean tolerations: description: Tolerations that are applied universally to server and sessions diff --git a/config/crd/bases/core.posit.team_workbenches.yaml b/config/crd/bases/core.posit.team_workbenches.yaml index d047931..d411d16 100644 --- a/config/crd/bases/core.posit.team_workbenches.yaml +++ b/config/crd/bases/core.posit.team_workbenches.yaml @@ -7574,6 +7574,11 @@ spec: clientId: type: string type: object + suspended: + description: |- + Suspended indicates Workbench should not run serving resources (Deployment, Service, Ingress) + but should preserve data resources (PVC, database, secrets). Set by the Site controller. + type: boolean tolerations: items: description: |- diff --git a/docs/api-reference.md b/docs/api-reference.md index 5616194..9c10a46 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -727,6 +727,8 @@ These types are used within the Site CRD for product configuration. | Field | Type | Description | |-------|------|-------------| +| `.enabled` | `*bool` | Controls whether Package Manager is running (default: `true`). Setting to `false` suspends Package Manager: stops pods and removes ingress/service, but preserves PVC, database, and secrets. | +| `.teardown` | `*bool` | Permanently destroys all Package Manager resources including database and PVC. Only takes effect when `enabled` is `false` (default: `false`). | | `.license` | `LicenseSpec` | License configuration | | `.volume` | `*VolumeSpec` | Data volume | | `.nodeSelector` | `map[string]string` | Node selector | @@ -767,6 +769,8 @@ These types are used within the Site CRD for product configuration. | Field | Type | Description | |-------|------|-------------| +| `.enabled` | `*bool` | Controls whether Workbench is running (default: `true`). Setting to `false` suspends Workbench: stops pods and removes ingress/service, but preserves PVC, database, and secrets. | +| `.teardown` | `*bool` | Permanently destroys all Workbench resources including database and PVC. Only takes effect when `enabled` is `false` (default: `false`). | | `.databricks` | `map[string]DatabricksConfig` | Databricks configurations | | `.snowflake` | `SnowflakeConfig` | Snowflake configuration | | `.license` | `LicenseSpec` | License configuration | @@ -801,6 +805,8 @@ These types are used within the Site CRD for product configuration. | Field | Type | Description | |-------|------|-------------| +| `.enabled` | `*bool` | Controls whether Chronicle is running (default: `true`). Setting to `false` suspends Chronicle: stops the StatefulSet and removes the service. | +| `.teardown` | `*bool` | Permanently destroys all Chronicle resources. Only takes effect when `enabled` is `false` (default: `false`). | | `.nodeSelector` | `map[string]string` | Node selector | | `.image` | `string` | Container image | | `.addEnv` | `map[string]string` | Environment variables | diff --git a/docs/guides/packagemanager-configuration.md b/docs/guides/packagemanager-configuration.md index cd80f40..de15b1d 100644 --- a/docs/guides/packagemanager-configuration.md +++ b/docs/guides/packagemanager-configuration.md @@ -25,6 +25,57 @@ When you configure Package Manager in a Site spec, the Site controller creates a ## Basic Configuration +### Enabling/Disabling Package Manager + +Package Manager can be suspended or permanently torn down using the `enabled` and `teardown` fields. + +#### Suspending Package Manager (non-destructive) + +Setting `enabled: false` suspends Package Manager: the Deployment, Service, and Ingress are removed, but the PVC, database, and secrets are preserved. Re-enabling restores full service with all existing data intact. + +```yaml +spec: + packageManager: + enabled: false # suspend — data is preserved +``` + +**When to use `enabled: false`:** + +- Customer does not have a Package Manager license yet — deploy the site without Package Manager and enable it once a license is purchased +- Temporarily pause Package Manager during a maintenance window or cost-saving period +- Stop Package Manager while retaining all package data and configuration for a possible return + +**Re-enabling Package Manager** after a suspend is as simple as removing the field or setting it back to `true`: + +```yaml +spec: + packageManager: + enabled: true # or omit the field entirely — defaults to true +``` + +#### Tearing down Package Manager (destructive) + +To permanently destroy all Package Manager resources — including the database, secrets, and PVC — set both `enabled: false` and `teardown: true`: + +```yaml +spec: + packageManager: + enabled: false + teardown: true # DESTRUCTIVE: deletes database, secrets, and PVC +``` + +**This is irreversible.** Re-enabling Package Manager after a teardown starts completely fresh with a new empty database and no prior package repositories or configuration. + +**When to use `teardown: true`:** + +- Permanently decommissioning Package Manager with no intent to restore data +- Reclaiming cluster storage after migrating to a different Package Manager instance +- Explicitly wiping Package Manager to start fresh + +> **Note:** `teardown: true` has no effect while `enabled` is `true` or unset. You must set `enabled: false` first. + +--- + ### Minimal Configuration ```yaml diff --git a/docs/guides/product-team-site-management.md b/docs/guides/product-team-site-management.md index 1f1d833..2c8967e 100644 --- a/docs/guides/product-team-site-management.md +++ b/docs/guides/product-team-site-management.md @@ -309,6 +309,12 @@ spec: ```yaml spec: workbench: + # Enable/disable Workbench deployment (default: true). + # Setting enabled: false suspends Workbench (preserves data). + # Use teardown: true to permanently delete all Workbench data. + # See the Workbench Configuration Guide for details. + enabled: true + image: "ghcr.io/posit-dev/workbench:jammy-2024.12.0" imagePullPolicy: IfNotPresent replicas: 1 @@ -416,6 +422,12 @@ spec: ```yaml spec: packageManager: + # Enable/disable Package Manager deployment (default: true). + # Setting enabled: false suspends Package Manager (preserves data). + # Use teardown: true to permanently delete all Package Manager data. + # See the Package Manager Configuration Guide for details. + enabled: true + image: "ghcr.io/posit-dev/package-manager:jammy-2024.08.0" imagePullPolicy: IfNotPresent replicas: 1 @@ -451,6 +463,12 @@ spec: ```yaml spec: chronicle: + # Enable/disable Chronicle deployment (default: true). + # Setting enabled: false suspends Chronicle. + # Use teardown: true to permanently delete all Chronicle data. + enabled: true + + image: "ghcr.io/posit-dev/chronicle:2024.11.0" imagePullPolicy: IfNotPresent diff --git a/docs/guides/workbench-configuration.md b/docs/guides/workbench-configuration.md index 9915e0e..feb3dcd 100644 --- a/docs/guides/workbench-configuration.md +++ b/docs/guides/workbench-configuration.md @@ -30,6 +30,57 @@ When configured via a Site resource, Workbench: ## Basic Configuration +### Enabling/Disabling Workbench + +Workbench can be suspended or permanently torn down using the `enabled` and `teardown` fields. + +#### Suspending Workbench (non-destructive) + +Setting `enabled: false` suspends Workbench: the Deployment, Service, and Ingress are removed, but the PVC, database, and secrets are preserved. Re-enabling restores full service with all existing data intact. + +```yaml +spec: + workbench: + enabled: false # suspend — data is preserved +``` + +**When to use `enabled: false`:** + +- Customer does not have a Workbench license yet — deploy the site without Workbench and enable it once a license is purchased +- Temporarily pause Workbench during a maintenance window or cost-saving period +- Stop Workbench while retaining all user home directories and configuration for a possible return + +**Re-enabling Workbench** after a suspend is as simple as removing the field or setting it back to `true`: + +```yaml +spec: + workbench: + enabled: true # or omit the field entirely — defaults to true +``` + +#### Tearing down Workbench (destructive) + +To permanently destroy all Workbench resources — including the database, secrets, and PVC — set both `enabled: false` and `teardown: true`: + +```yaml +spec: + workbench: + enabled: false + teardown: true # DESTRUCTIVE: deletes database, secrets, and PVC +``` + +**This is irreversible.** Re-enabling Workbench after a teardown starts completely fresh with a new empty database and no prior user home directories or configuration. + +**When to use `teardown: true`:** + +- Permanently decommissioning Workbench with no intent to restore data +- Reclaiming cluster storage after migrating to a different Workbench instance +- Explicitly wiping Workbench to start fresh + +> **Note:** `teardown: true` has no effect while `enabled` is `true` or unset. You must set `enabled: false` first. + +--- + ### Image and Resources Configure the Workbench server image and basic settings: diff --git a/internal/controller/core/chronicle_controller.go b/internal/controller/core/chronicle_controller.go index 82f6de4..615f9da 100644 --- a/internal/controller/core/chronicle_controller.go +++ b/internal/controller/core/chronicle_controller.go @@ -99,6 +99,11 @@ func (r *ChronicleReconciler) ReconcileChronicle(ctx context.Context, req ctrl.R "product", "chronicle", ) + // If suspended, clean up serving resources but preserve configuration + if c.Spec.Suspended != nil && *c.Spec.Suspended { + return r.suspendDeployedService(ctx, req, c) + } + // default config settings not in the original object // ... @@ -328,7 +333,64 @@ func (r *ChronicleReconciler) ensureDeployedService(ctx context.Context, req ctr } func (r *ChronicleReconciler) CleanupChronicle(ctx context.Context, req ctrl.Request, c *positcov1beta1.Chronicle) (ctrl.Result, error) { - // TODO: some cleanup...? + l := r.GetLogger(ctx).WithValues( + "event", "cleanup-chronicle", + "product", "chronicle", + ) + + key := client.ObjectKey{Name: c.ComponentName(), Namespace: req.Namespace} + + // SERVICE + if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { + return ctrl.Result{}, err + } + + // STATEFULSET + if err := internal.BasicDelete(ctx, r, l, key, &v1.StatefulSet{}); err != nil { + return ctrl.Result{}, err + } + + // CONFIGMAP + if err := internal.BasicDelete(ctx, r, l, key, &corev1.ConfigMap{}); err != nil { + return ctrl.Result{}, err + } + + // SERVICE ACCOUNTS + if err := internal.BasicDelete(ctx, r, l, key, &corev1.ServiceAccount{}); err != nil { + return ctrl.Result{}, err + } + + // Read-only service account + readOnlyKey := client.ObjectKey{ + Name: fmt.Sprintf("%s-read-only", c.ComponentName()), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, readOnlyKey, &corev1.ServiceAccount{}); err != nil { + return ctrl.Result{}, err + } + + l.Info("Chronicle cleanup complete") + return ctrl.Result{}, nil +} + +// suspendDeployedService removes serving resources (StatefulSet, Service) +// when Chronicle is suspended. +func (r *ChronicleReconciler) suspendDeployedService(ctx context.Context, req ctrl.Request, c *positcov1beta1.Chronicle) (ctrl.Result, error) { + l := r.GetLogger(ctx).WithValues("event", "suspend-service", "product", "chronicle") + + key := client.ObjectKey{Name: c.ComponentName(), Namespace: req.Namespace} + + // SERVICE + if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { + return ctrl.Result{}, err + } + + // STATEFULSET (Chronicle uses StatefulSet, not Deployment) + if err := internal.BasicDelete(ctx, r, l, key, &v1.StatefulSet{}); err != nil { + return ctrl.Result{}, err + } + + l.Info("Chronicle serving resources suspended") return ctrl.Result{}, nil } diff --git a/internal/controller/core/package_manager.go b/internal/controller/core/package_manager.go index af2fef6..2cbf1b5 100644 --- a/internal/controller/core/package_manager.go +++ b/internal/controller/core/package_manager.go @@ -45,6 +45,32 @@ func (r *PackageManagerReconciler) CleanupPackageManager(ctx context.Context, re return ctrl.Result{}, nil } +// suspendDeployedService removes serving resources (Deployment, Service, Ingress) +// while preserving data resources (PVC, database, secrets) when Package Manager is suspended. +func (r *PackageManagerReconciler) suspendDeployedService(ctx context.Context, req ctrl.Request, pm *positcov1beta1.PackageManager) (ctrl.Result, error) { + l := r.GetLogger(ctx).WithValues("event", "suspend-service", "product", "package-manager") + + key := client.ObjectKey{Name: pm.ComponentName(), Namespace: req.Namespace} + + // INGRESS + if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.Ingress{}); err != nil { + return ctrl.Result{}, err + } + + // SERVICE + if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { + return ctrl.Result{}, err + } + + // DEPLOYMENT + if err := internal.BasicDelete(ctx, r, l, key, &v1.Deployment{}); err != nil { + return ctrl.Result{}, err + } + + l.Info("Package Manager serving resources suspended") + return ctrl.Result{}, nil +} + func (r *PackageManagerReconciler) cleanupDeployedService(ctx context.Context, req ctrl.Request, pm *positcov1beta1.PackageManager) error { l := r.GetLogger(ctx).WithValues( "event", "cleanup-service", @@ -113,6 +139,11 @@ func (r *PackageManagerReconciler) ReconcilePackageManager(ctx context.Context, "product", "package-manager", ) + // If suspended, clean up serving resources but preserve data + if pm.Spec.Suspended != nil && *pm.Spec.Suspended { + return r.suspendDeployedService(ctx, req, pm) + } + // create database secretKey := "pkg-db-password" if err := db.EnsureDatabaseExists(ctx, r, req, pm, pm.Spec.DatabaseConfig, pm.ComponentName(), "", []string{"pm", "metrics"}, pm.Spec.Secret, pm.Spec.WorkloadSecret, pm.Spec.MainDatabaseCredentialSecret, secretKey); err != nil { diff --git a/internal/controller/core/site_controller.go b/internal/controller/core/site_controller.go index 441df34..bdb9503 100644 --- a/internal/controller/core/site_controller.go +++ b/internal/controller/core/site_controller.go @@ -162,6 +162,24 @@ func (r *SiteReconciler) reconcileResources(ctx context.Context, req ctrl.Reques l.Info("connect.teardown is set but connect.enabled is not false; teardown has no effect until enabled=false") } + workbenchEnabled := site.Spec.Workbench.Enabled == nil || *site.Spec.Workbench.Enabled + workbenchTeardown := site.Spec.Workbench.Teardown != nil && *site.Spec.Workbench.Teardown + if workbenchTeardown && workbenchEnabled { + l.Info("workbench.teardown is set but workbench.enabled is not false; teardown has no effect until enabled=false") + } + + pmEnabled := site.Spec.PackageManager.Enabled == nil || *site.Spec.PackageManager.Enabled + pmTeardown := site.Spec.PackageManager.Teardown != nil && *site.Spec.PackageManager.Teardown + if pmTeardown && pmEnabled { + l.Info("packageManager.teardown is set but packageManager.enabled is not false; teardown has no effect until enabled=false") + } + + chronicleEnabled := site.Spec.Chronicle.Enabled == nil || *site.Spec.Chronicle.Enabled + chronicleTeardown := site.Spec.Chronicle.Teardown != nil && *site.Spec.Chronicle.Teardown + if chronicleTeardown && chronicleEnabled { + l.Info("chronicle.teardown is set but chronicle.enabled is not false; teardown has no effect until enabled=false") + } + connectVolumeName := fmt.Sprintf("%s-connect", site.Name) connectStorageClassName := connectVolumeName devVolumeName := fmt.Sprintf("%s-workbench", site.Name) @@ -191,15 +209,17 @@ func (r *SiteReconciler) reconcileResources(ctx context.Context, req ctrl.Reques } } - if err := r.provisionFsxVolume(ctx, site, devVolumeName, "workbench", connectVolumeSize); err != nil { - return ctrl.Result{}, err - } + if workbenchEnabled { + if err := r.provisionFsxVolume(ctx, site, devVolumeName, "workbench", connectVolumeSize); err != nil { + return ctrl.Result{}, err + } - // Provision shared storage volume for workbench load balancing - workbenchSharedStorageVolumeName := fmt.Sprintf("%s-workbench-shared-storage", site.Name) - // Note: provisionFsxVolume uses the volume name as the storage class name - if err := r.provisionFsxVolume(ctx, site, workbenchSharedStorageVolumeName, "workbench-shared-storage", workbenchSharedStorageVolumeSize); err != nil { - return ctrl.Result{}, err + // Provision shared storage volume for workbench load balancing + workbenchSharedStorageVolumeName := fmt.Sprintf("%s-workbench-shared-storage", site.Name) + // Note: provisionFsxVolume uses the volume name as the storage class name + if err := r.provisionFsxVolume(ctx, site, workbenchSharedStorageVolumeName, "workbench-shared-storage", workbenchSharedStorageVolumeSize); err != nil { + return ctrl.Result{}, err + } } if site.Spec.SharedDirectory != "" { @@ -230,15 +250,17 @@ func (r *SiteReconciler) reconcileResources(ctx context.Context, req ctrl.Reques devStorageClassName = fmt.Sprintf("%s-nfs", devVolumeName) - if err := r.provisionNfsVolume(ctx, site, devVolumeName, "workbench", devStorageClassName, connectVolumeSize); err != nil { - return ctrl.Result{}, err - } + if workbenchEnabled { + if err := r.provisionNfsVolume(ctx, site, devVolumeName, "workbench", devStorageClassName, connectVolumeSize); err != nil { + return ctrl.Result{}, err + } - // Provision shared storage volume for workbench load balancing - workbenchSharedStorageVolumeName := fmt.Sprintf("%s-workbench-shared-storage", site.Name) - workbenchSharedStorageClassName := fmt.Sprintf("%s-nfs", workbenchSharedStorageVolumeName) - if err := r.provisionNfsVolume(ctx, site, workbenchSharedStorageVolumeName, "workbench-shared-storage", workbenchSharedStorageClassName, workbenchSharedStorageVolumeSize); err != nil { - return ctrl.Result{}, err + // Provision shared storage volume for workbench load balancing + workbenchSharedStorageVolumeName := fmt.Sprintf("%s-workbench-shared-storage", site.Name) + workbenchSharedStorageClassName := fmt.Sprintf("%s-nfs", workbenchSharedStorageVolumeName) + if err := r.provisionNfsVolume(ctx, site, workbenchSharedStorageVolumeName, "workbench-shared-storage", workbenchSharedStorageClassName, workbenchSharedStorageVolumeSize); err != nil { + return ctrl.Result{}, err + } } if site.Spec.SharedDirectory != "" { @@ -344,40 +366,75 @@ func (r *SiteReconciler) reconcileResources(ctx context.Context, req ctrl.Reques } // PACKAGE MANAGER - if err := r.reconcilePackageManager( - ctx, - req, - site, - dbUrl.Host, - sslMode, - packageManagerUrl, - ); err != nil { - l.Error(err, "error reconciling package manager") - return ctrl.Result{}, err + if pmEnabled { + if err := r.reconcilePackageManager( + ctx, + req, + site, + dbUrl.Host, + sslMode, + packageManagerUrl, + ); err != nil { + l.Error(err, "error reconciling package manager") + return ctrl.Result{}, err + } + } else if pmTeardown { + if err := r.cleanupPackageManager(ctx, req, l); err != nil { + l.Error(err, "error tearing down package manager resources") + return ctrl.Result{}, err + } + } else { + if err := r.disablePackageManager(ctx, req, l); err != nil { + l.Error(err, "error disabling package manager") + return ctrl.Result{}, err + } } // WORKBENCH - if err := r.reconcileWorkbench( - ctx, - req, - site, - dbUrl.Host, - sslMode, - devVolumeName, - devStorageClassName, - workbenchAdditionalVolumes, - packageManagerRepoUrl, - workbenchUrl, - ); err != nil { - l.Error(err, "error reconciling workbench") - return ctrl.Result{}, err + if workbenchEnabled { + if err := r.reconcileWorkbench( + ctx, + req, + site, + dbUrl.Host, + sslMode, + devVolumeName, + devStorageClassName, + workbenchAdditionalVolumes, + packageManagerRepoUrl, + workbenchUrl, + ); err != nil { + l.Error(err, "error reconciling workbench") + return ctrl.Result{}, err + } + } else if workbenchTeardown { + if err := r.cleanupWorkbench(ctx, req, l); err != nil { + l.Error(err, "error tearing down workbench resources") + return ctrl.Result{}, err + } + } else { + if err := r.disableWorkbench(ctx, req, l); err != nil { + l.Error(err, "error disabling workbench") + return ctrl.Result{}, err + } } // CHRONICLE - - if err := r.reconcileChronicle(ctx, req, site); err != nil { - l.Error(err, "error reconciling chronicle") - return ctrl.Result{}, err + if chronicleEnabled { + if err := r.reconcileChronicle(ctx, req, site); err != nil { + l.Error(err, "error reconciling chronicle") + return ctrl.Result{}, err + } + } else if chronicleTeardown { + if err := r.cleanupChronicle(ctx, req, l); err != nil { + l.Error(err, "error tearing down chronicle resources") + return ctrl.Result{}, err + } + } else { + if err := r.disableChronicle(ctx, req, l); err != nil { + l.Error(err, "error disabling chronicle") + return ctrl.Result{}, err + } } // KEYCLOAK diff --git a/internal/controller/core/site_controller_chronicle.go b/internal/controller/core/site_controller_chronicle.go index c901789..09c479f 100644 --- a/internal/controller/core/site_controller_chronicle.go +++ b/internal/controller/core/site_controller_chronicle.go @@ -3,11 +3,14 @@ package core import ( "context" + "github.com/go-logr/logr" "github.com/posit-dev/team-operator/api/core/v1beta1" "github.com/posit-dev/team-operator/api/product" "github.com/posit-dev/team-operator/internal" + apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) func (r *SiteReconciler) reconcileChronicle(ctx context.Context, req controllerruntime.Request, site *v1beta1.Site) error { @@ -93,3 +96,61 @@ func (r *SiteReconciler) reconcileChronicle(ctx context.Context, req controllerr } return nil } + +// disableChronicle suspends Chronicle by marking the existing Chronicle CR with Suspended=true. +// The Chronicle controller then removes serving resources (Deployment/Service/Ingress) while +// preserving any configuration. +// +// If no Chronicle CR exists yet (Chronicle was never enabled), this is a no-op. +// When Chronicle is re-enabled, reconcileChronicle overwrites Suspended back to nil and +// performs a full reconcile. +func (r *SiteReconciler) disableChronicle(ctx context.Context, req controllerruntime.Request, l logr.Logger) error { + l = l.WithValues("event", "disable-chronicle") + + chronicle := &v1beta1.Chronicle{} + if err := r.Get(ctx, client.ObjectKey{Name: req.Name, Namespace: req.Namespace}, chronicle); err != nil { + if apierrors.IsNotFound(err) { + l.Info("Chronicle CR not found, nothing to suspend") + return nil + } + return err + } + + if chronicle.Spec.Suspended != nil && *chronicle.Spec.Suspended { + l.Info("Chronicle already suspended") + return nil + } + + patch := client.MergeFrom(chronicle.DeepCopy()) + suspended := true + chronicle.Spec.Suspended = &suspended + if err := r.Patch(ctx, chronicle, patch); err != nil { + l.Error(err, "error suspending Chronicle CR") + return err + } + + l.Info("Chronicle CR suspended") + return nil +} + +// cleanupChronicle deletes the Chronicle CRD when teardown=true. +// +// WARNING: This is a DESTRUCTIVE operation. Deleting the Chronicle CRD triggers the Chronicle +// finalizer which permanently destroys: +// - All deployed Kubernetes resources +// - Chronicle storage (S3 data or local volumes) +// +// Note: Chronicle does not use a database or persistent volumes like other products. +// +// This is triggered by Site.Spec.Chronicle.Teardown=true (when Enabled=false). +// Re-enabling Chronicle after teardown will start fresh. +func (r *SiteReconciler) cleanupChronicle(ctx context.Context, req controllerruntime.Request, l logr.Logger) error { + l = l.WithValues("event", "cleanup-chronicle") + + chronicleKey := client.ObjectKey{Name: req.Name, Namespace: req.Namespace} + if err := internal.BasicDelete(ctx, r, l, chronicleKey, &v1beta1.Chronicle{}); err != nil { + return err + } + + return nil +} diff --git a/internal/controller/core/site_controller_networkpolicies.go b/internal/controller/core/site_controller_networkpolicies.go index c98222c..e5d6ae0 100644 --- a/internal/controller/core/site_controller_networkpolicies.go +++ b/internal/controller/core/site_controller_networkpolicies.go @@ -38,9 +38,18 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. return nil } - if err := r.reconcileChronicleNetworkPolicy(ctx, req.Namespace, l, site); err != nil { - l.Error(err, "error ensuring chronicle network policy") - return err + // Chronicle network policy + chronicleEnabled := site.Spec.Chronicle.Enabled == nil || *site.Spec.Chronicle.Enabled + if chronicleEnabled { + if err := r.reconcileChronicleNetworkPolicy(ctx, req.Namespace, l, site); err != nil { + l.Error(err, "error ensuring chronicle network policy") + return err + } + } else { + if err := r.cleanupChronicleNetworkPolicies(ctx, req, l); err != nil { + l.Error(err, "error cleaning up chronicle network policies") + return err + } } // Connect network policies @@ -72,19 +81,37 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. return err } - if err := r.reconcilePackageManagerNetworkPolicy(ctx, req.Namespace, l, site); err != nil { - l.Error(err, "error ensuring package manager network policy") - return err + // Package Manager network policy + pmEnabled := site.Spec.PackageManager.Enabled == nil || *site.Spec.PackageManager.Enabled + if pmEnabled { + if err := r.reconcilePackageManagerNetworkPolicy(ctx, req.Namespace, l, site); err != nil { + l.Error(err, "error ensuring package manager network policy") + return err + } + } else { + if err := r.cleanupPackageManagerNetworkPolicies(ctx, req, l); err != nil { + l.Error(err, "error cleaning up package manager network policies") + return err + } } - if err := r.reconcileWorkbenchNetworkPolicy(ctx, req.Namespace, l, site); err != nil { - l.Error(err, "error ensuring workbench network policy") - return err - } + // Workbench network policies + workbenchEnabled := site.Spec.Workbench.Enabled == nil || *site.Spec.Workbench.Enabled + if workbenchEnabled { + if err := r.reconcileWorkbenchNetworkPolicy(ctx, req.Namespace, l, site); err != nil { + l.Error(err, "error ensuring workbench network policy") + return err + } - if err := r.reconcileWorkbenchSessionNetworkPolicy(ctx, req.Namespace, l, site); err != nil { - l.Error(err, "error ensuring workbench session network policy") - return err + if err := r.reconcileWorkbenchSessionNetworkPolicy(ctx, req.Namespace, l, site); err != nil { + l.Error(err, "error ensuring workbench session network policy") + return err + } + } else { + if err := r.cleanupWorkbenchNetworkPolicies(ctx, req, l); err != nil { + l.Error(err, "error cleaning up workbench network policies") + return err + } } if err := r.reconcileFlightdeckNetworkPolicy(ctx, req.Namespace, l, site); err != nil { @@ -802,3 +829,23 @@ func (r *SiteReconciler) cleanupConnectNetworkPolicies(ctx context.Context, req } return nil } + +func (r *SiteReconciler) cleanupChronicleNetworkPolicies(ctx context.Context, req ctrl.Request, l logr.Logger) error { + key := client.ObjectKey{Name: req.Name + "-chronicle", Namespace: req.Namespace} + return internal.BasicDelete(ctx, r, l, key, &networkingv1.NetworkPolicy{}) +} + +func (r *SiteReconciler) cleanupWorkbenchNetworkPolicies(ctx context.Context, req ctrl.Request, l logr.Logger) error { + for _, suffix := range []string{"workbench", "workbench-session"} { + key := client.ObjectKey{Name: req.Name + "-" + suffix, Namespace: req.Namespace} + if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.NetworkPolicy{}); err != nil { + return err + } + } + return nil +} + +func (r *SiteReconciler) cleanupPackageManagerNetworkPolicies(ctx context.Context, req ctrl.Request, l logr.Logger) error { + key := client.ObjectKey{Name: req.Name + "-packagemanager", Namespace: req.Namespace} + return internal.BasicDelete(ctx, r, l, key, &networkingv1.NetworkPolicy{}) +} diff --git a/internal/controller/core/site_controller_package_manager.go b/internal/controller/core/site_controller_package_manager.go index bca0a64..482b9aa 100644 --- a/internal/controller/core/site_controller_package_manager.go +++ b/internal/controller/core/site_controller_package_manager.go @@ -3,11 +3,14 @@ package core import ( "context" + "github.com/go-logr/logr" "github.com/posit-dev/team-operator/api/core/v1beta1" "github.com/posit-dev/team-operator/api/product" "github.com/posit-dev/team-operator/internal" + apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) func (r *SiteReconciler) reconcilePackageManager( @@ -125,3 +128,61 @@ func (r *SiteReconciler) reconcilePackageManager( return nil } + +// disablePackageManager suspends Package Manager by marking the existing PackageManager CR with Suspended=true. +// The Package Manager controller then removes serving resources (Deployment/Service/Ingress) while +// preserving data resources (PVC, database, secrets). +// +// If no PackageManager CR exists yet (Package Manager was never enabled), this is a no-op. +// When Package Manager is re-enabled, reconcilePackageManager overwrites Suspended back to nil and +// performs a full reconcile. +func (r *SiteReconciler) disablePackageManager(ctx context.Context, req controllerruntime.Request, l logr.Logger) error { + l = l.WithValues("event", "disable-package-manager") + + pm := &v1beta1.PackageManager{} + if err := r.Get(ctx, client.ObjectKey{Name: req.Name, Namespace: req.Namespace}, pm); err != nil { + if apierrors.IsNotFound(err) { + l.Info("PackageManager CR not found, nothing to suspend") + return nil + } + return err + } + + if pm.Spec.Suspended != nil && *pm.Spec.Suspended { + l.Info("PackageManager already suspended") + return nil + } + + patch := client.MergeFrom(pm.DeepCopy()) + suspended := true + pm.Spec.Suspended = &suspended + if err := r.Patch(ctx, pm, patch); err != nil { + l.Error(err, "error suspending PackageManager CR") + return err + } + + l.Info("PackageManager CR suspended") + return nil +} + +// cleanupPackageManager deletes the PackageManager CRD when teardown=true. +// +// WARNING: This is a DESTRUCTIVE operation. Deleting the PackageManager CRD triggers the PackageManager +// finalizer which permanently destroys: +// - The Package Manager database and all its data +// - All secrets (database credentials, provisioning keys, etc.) +// - Persistent volumes and claims +// - All deployed Kubernetes resources +// +// This is triggered by Site.Spec.PackageManager.Teardown=true (when Enabled=false). +// Re-enabling Package Manager after teardown will start fresh with a new database. +func (r *SiteReconciler) cleanupPackageManager(ctx context.Context, req controllerruntime.Request, l logr.Logger) error { + l = l.WithValues("event", "cleanup-package-manager") + + pmKey := client.ObjectKey{Name: req.Name, Namespace: req.Namespace} + if err := internal.BasicDelete(ctx, r, l, pmKey, &v1beta1.PackageManager{}); err != nil { + return err + } + + return nil +} diff --git a/internal/controller/core/site_controller_workbench.go b/internal/controller/core/site_controller_workbench.go index 428218e..8a002d6 100644 --- a/internal/controller/core/site_controller_workbench.go +++ b/internal/controller/core/site_controller_workbench.go @@ -5,13 +5,16 @@ import ( "fmt" "strings" + "github.com/go-logr/logr" "github.com/posit-dev/team-operator/api/core/v1beta1" "github.com/posit-dev/team-operator/api/product" "github.com/posit-dev/team-operator/internal" "github.com/rstudio/goex/ptr" v12 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) func (r *SiteReconciler) reconcileWorkbench( @@ -528,3 +531,61 @@ func getMemoryRequestRatio(experimentalFeatures *v1beta1.InternalWorkbenchExperi } return "0.8" // Default when experimentalFeatures is nil or field is empty (kubebuilder sets this for new resources) } + +// disableWorkbench suspends Workbench by marking the existing Workbench CR with Suspended=true. +// The Workbench controller then removes serving resources (Deployment/Service/Ingress) while +// preserving data resources (PVC, database, secrets). +// +// If no Workbench CR exists yet (Workbench was never enabled), this is a no-op. +// When Workbench is re-enabled, reconcileWorkbench overwrites Suspended back to nil and +// performs a full reconcile. +func (r *SiteReconciler) disableWorkbench(ctx context.Context, req controllerruntime.Request, l logr.Logger) error { + l = l.WithValues("event", "disable-workbench") + + workbench := &v1beta1.Workbench{} + if err := r.Get(ctx, client.ObjectKey{Name: req.Name, Namespace: req.Namespace}, workbench); err != nil { + if apierrors.IsNotFound(err) { + l.Info("Workbench CR not found, nothing to suspend") + return nil + } + return err + } + + if workbench.Spec.Suspended != nil && *workbench.Spec.Suspended { + l.Info("Workbench already suspended") + return nil + } + + patch := client.MergeFrom(workbench.DeepCopy()) + suspended := true + workbench.Spec.Suspended = &suspended + if err := r.Patch(ctx, workbench, patch); err != nil { + l.Error(err, "error suspending Workbench CR") + return err + } + + l.Info("Workbench CR suspended") + return nil +} + +// cleanupWorkbench deletes the Workbench CRD when teardown=true. +// +// WARNING: This is a DESTRUCTIVE operation. Deleting the Workbench CRD triggers the Workbench +// finalizer which permanently destroys: +// - The Workbench database and all its data +// - All secrets (database credentials, provisioning keys, etc.) +// - Persistent volumes and claims +// - All deployed Kubernetes resources +// +// This is triggered by Site.Spec.Workbench.Teardown=true (when Enabled=false). +// Re-enabling Workbench after teardown will start fresh with a new database. +func (r *SiteReconciler) cleanupWorkbench(ctx context.Context, req controllerruntime.Request, l logr.Logger) error { + l = l.WithValues("event", "cleanup-workbench") + + workbenchKey := client.ObjectKey{Name: req.Name, Namespace: req.Namespace} + if err := internal.BasicDelete(ctx, r, l, workbenchKey, &v1beta1.Workbench{}); err != nil { + return err + } + + return nil +} diff --git a/internal/controller/core/site_test.go b/internal/controller/core/site_test.go index ec91004..57a989b 100644 --- a/internal/controller/core/site_test.go +++ b/internal/controller/core/site_test.go @@ -323,6 +323,13 @@ func getPackageManager(t *testing.T, cli client.Client, siteNamespace, siteName return pm } +func getChronicle(t *testing.T, cli client.Client, siteNamespace, siteName string) *v1beta1.Chronicle { + chronicle := &v1beta1.Chronicle{} + err := cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle, &client.GetOptions{}) + assert.Nil(t, err) + return chronicle +} + func getFlightdeck(t *testing.T, cli client.Client, siteNamespace, siteName string) *v1beta1.Flightdeck { fd := &v1beta1.Flightdeck{} err := cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, fd, &client.GetOptions{}) @@ -1249,3 +1256,288 @@ func TestSiteConnectTeardown(t *testing.T) { err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, connect) assert.Error(t, err, "Connect CR should not exist after teardown=true") } + +// TestSiteWorkbenchDisableNeverEnabled verifies that setting enabled=false when Workbench was +// never enabled is a no-op: no Workbench CR is created. +func TestSiteWorkbenchDisableNeverEnabled(t *testing.T) { + siteName := "never-enabled-workbench" + siteNamespace := "posit-team" + site := defaultSite(siteName) + enabled := false + site.Spec.Workbench.Enabled = &enabled + + cli, _, err := runFakeSiteReconciler(t, siteNamespace, siteName, site) + assert.NoError(t, err) + + // Workbench CR should NOT exist — disable with no prior enablement is a no-op + workbench := &v1beta1.Workbench{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, workbench) + assert.Error(t, err, "expected Workbench CR to not exist when disabled without ever being enabled") +} + +// TestSiteWorkbenchSuspendAfterEnable verifies that setting enabled=false after Workbench was running +// suspends the Workbench CR (Suspended=true) rather than deleting it, preserving data. +// It also verifies that re-enabling clears Suspended and restores full reconciliation. +func TestSiteWorkbenchSuspendAfterEnable(t *testing.T) { + siteName := "suspend-workbench" + siteNamespace := "posit-team" + + // Share a single fake environment across all reconcile passes. + fakeClient := localtest.FakeTestEnv{} + cli, scheme, log := fakeClient.Start(loadSchemes) + rec := SiteReconciler{Client: cli, Scheme: scheme, Log: log} + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: siteNamespace, Name: siteName}} + + // Pass 1: Workbench enabled (default) + site := defaultSite(siteName) + _, err := rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + workbench := &v1beta1.Workbench{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, workbench) + assert.NoError(t, err, "Workbench CR should exist after first reconcile") + assert.Nil(t, workbench.Spec.Suspended) + + // Pass 2: disable Workbench without teardown — Suspended should be true + enabled := false + site.Spec.Workbench.Enabled = &enabled + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, workbench) + assert.NoError(t, err, "Workbench CR should still exist when disabled without teardown") + assert.NotNil(t, workbench.Spec.Suspended) + assert.True(t, *workbench.Spec.Suspended) + + // Pass 3: re-enable Workbench — Suspended should be cleared + site.Spec.Workbench.Enabled = nil + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, workbench) + assert.NoError(t, err, "Workbench CR should still exist after re-enable") + assert.Nil(t, workbench.Spec.Suspended, "Suspended should be cleared after re-enable") +} + +// TestSiteWorkbenchTeardown verifies that setting enabled=false + teardown=true causes the +// Workbench CR to be deleted (triggering the destructive finalizer path). +func TestSiteWorkbenchTeardown(t *testing.T) { + siteName := "teardown-workbench" + siteNamespace := "posit-team" + + fakeClient := localtest.FakeTestEnv{} + cli, scheme, log := fakeClient.Start(loadSchemes) + rec := SiteReconciler{Client: cli, Scheme: scheme, Log: log} + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: siteNamespace, Name: siteName}} + + // Pass 1: establish a running Workbench CR + site := defaultSite(siteName) + _, err := rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + workbench := &v1beta1.Workbench{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, workbench) + assert.NoError(t, err, "Workbench CR should exist before teardown") + + // Pass 2: teardown + enabled := false + teardown := true + site.Spec.Workbench.Enabled = &enabled + site.Spec.Workbench.Teardown = &teardown + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + // Workbench CR should NOT exist after teardown + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, workbench) + assert.Error(t, err, "Workbench CR should not exist after teardown=true") +} + +// TestSitePackageManagerDisableNeverEnabled verifies that setting enabled=false when Package Manager was +// never enabled is a no-op: no PackageManager CR is created. +func TestSitePackageManagerDisableNeverEnabled(t *testing.T) { + siteName := "never-enabled-pm" + siteNamespace := "posit-team" + site := defaultSite(siteName) + enabled := false + site.Spec.PackageManager.Enabled = &enabled + + cli, _, err := runFakeSiteReconciler(t, siteNamespace, siteName, site) + assert.NoError(t, err) + + // PackageManager CR should NOT exist — disable with no prior enablement is a no-op + pm := &v1beta1.PackageManager{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, pm) + assert.Error(t, err, "expected PackageManager CR to not exist when disabled without ever being enabled") +} + +// TestSitePackageManagerSuspendAfterEnable verifies that setting enabled=false after Package Manager was running +// suspends the PackageManager CR (Suspended=true) rather than deleting it, preserving data. +// It also verifies that re-enabling clears Suspended and restores full reconciliation. +func TestSitePackageManagerSuspendAfterEnable(t *testing.T) { + siteName := "suspend-pm" + siteNamespace := "posit-team" + + // Share a single fake environment across all reconcile passes. + fakeClient := localtest.FakeTestEnv{} + cli, scheme, log := fakeClient.Start(loadSchemes) + rec := SiteReconciler{Client: cli, Scheme: scheme, Log: log} + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: siteNamespace, Name: siteName}} + + // Pass 1: PackageManager enabled (default) + site := defaultSite(siteName) + _, err := rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + pm := &v1beta1.PackageManager{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, pm) + assert.NoError(t, err, "PackageManager CR should exist after first reconcile") + assert.Nil(t, pm.Spec.Suspended) + + // Pass 2: disable PackageManager without teardown — Suspended should be true + enabled := false + site.Spec.PackageManager.Enabled = &enabled + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, pm) + assert.NoError(t, err, "PackageManager CR should still exist when disabled without teardown") + assert.NotNil(t, pm.Spec.Suspended) + assert.True(t, *pm.Spec.Suspended) + + // Pass 3: re-enable PackageManager — Suspended should be cleared + site.Spec.PackageManager.Enabled = nil + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, pm) + assert.NoError(t, err, "PackageManager CR should still exist after re-enable") + assert.Nil(t, pm.Spec.Suspended, "Suspended should be cleared after re-enable") +} + +// TestSitePackageManagerTeardown verifies that setting enabled=false + teardown=true causes the +// PackageManager CR to be deleted (triggering the destructive finalizer path). +func TestSitePackageManagerTeardown(t *testing.T) { + siteName := "teardown-pm" + siteNamespace := "posit-team" + + fakeClient := localtest.FakeTestEnv{} + cli, scheme, log := fakeClient.Start(loadSchemes) + rec := SiteReconciler{Client: cli, Scheme: scheme, Log: log} + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: siteNamespace, Name: siteName}} + + // Pass 1: establish a running PackageManager CR + site := defaultSite(siteName) + _, err := rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + pm := &v1beta1.PackageManager{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, pm) + assert.NoError(t, err, "PackageManager CR should exist before teardown") + + // Pass 2: teardown + enabled := false + teardown := true + site.Spec.PackageManager.Enabled = &enabled + site.Spec.PackageManager.Teardown = &teardown + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + // PackageManager CR should NOT exist after teardown + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, pm) + assert.Error(t, err, "PackageManager CR should not exist after teardown=true") +} + +// TestSiteChronicleDisableNeverEnabled verifies that setting enabled=false when Chronicle was +// never enabled is a no-op: no Chronicle CR is created. +func TestSiteChronicleDisableNeverEnabled(t *testing.T) { + siteName := "never-enabled-chronicle" + siteNamespace := "posit-team" + site := defaultSite(siteName) + enabled := false + site.Spec.Chronicle.Enabled = &enabled + + cli, _, err := runFakeSiteReconciler(t, siteNamespace, siteName, site) + assert.NoError(t, err) + + // Chronicle CR should NOT exist — disable with no prior enablement is a no-op + chronicle := &v1beta1.Chronicle{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) + assert.Error(t, err, "expected Chronicle CR to not exist when disabled without ever being enabled") +} + +// TestSiteChronicleSuspendAfterEnable verifies that setting enabled=false after Chronicle was running +// suspends the Chronicle CR (Suspended=true) rather than deleting it, preserving data. +// It also verifies that re-enabling clears Suspended and restores full reconciliation. +func TestSiteChronicleSuspendAfterEnable(t *testing.T) { + siteName := "suspend-chronicle" + siteNamespace := "posit-team" + + // Share a single fake environment across all reconcile passes. + fakeClient := localtest.FakeTestEnv{} + cli, scheme, log := fakeClient.Start(loadSchemes) + rec := SiteReconciler{Client: cli, Scheme: scheme, Log: log} + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: siteNamespace, Name: siteName}} + + // Pass 1: Chronicle enabled (default) + site := defaultSite(siteName) + _, err := rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + chronicle := &v1beta1.Chronicle{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) + assert.NoError(t, err, "Chronicle CR should exist after first reconcile") + assert.Nil(t, chronicle.Spec.Suspended) + + // Pass 2: disable Chronicle without teardown — Suspended should be true + enabled := false + site.Spec.Chronicle.Enabled = &enabled + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) + assert.NoError(t, err, "Chronicle CR should still exist when disabled without teardown") + assert.NotNil(t, chronicle.Spec.Suspended) + assert.True(t, *chronicle.Spec.Suspended) + + // Pass 3: re-enable Chronicle — Suspended should be cleared + site.Spec.Chronicle.Enabled = nil + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) + assert.NoError(t, err, "Chronicle CR should still exist after re-enable") + assert.Nil(t, chronicle.Spec.Suspended, "Suspended should be cleared after re-enable") +} + +// TestSiteChronicleTeardown verifies that setting enabled=false + teardown=true causes the +// Chronicle CR to be deleted (triggering the destructive finalizer path). +func TestSiteChronicleTeardown(t *testing.T) { + siteName := "teardown-chronicle" + siteNamespace := "posit-team" + + fakeClient := localtest.FakeTestEnv{} + cli, scheme, log := fakeClient.Start(loadSchemes) + rec := SiteReconciler{Client: cli, Scheme: scheme, Log: log} + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: siteNamespace, Name: siteName}} + + // Pass 1: establish a running Chronicle CR + site := defaultSite(siteName) + _, err := rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + chronicle := &v1beta1.Chronicle{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) + assert.NoError(t, err, "Chronicle CR should exist before teardown") + + // Pass 2: teardown + enabled := false + teardown := true + site.Spec.Chronicle.Enabled = &enabled + site.Spec.Chronicle.Teardown = &teardown + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + // Chronicle CR should NOT exist after teardown + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) + assert.Error(t, err, "Chronicle CR should not exist after teardown=true") +} diff --git a/internal/controller/core/workbench.go b/internal/controller/core/workbench.go index 721c9d5..3065565 100644 --- a/internal/controller/core/workbench.go +++ b/internal/controller/core/workbench.go @@ -18,6 +18,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" + rbacv1 "k8s.io/api/rbac/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -76,6 +77,11 @@ func (r *WorkbenchReconciler) ReconcileWorkbench(ctx context.Context, req ctrl.R "product", "workbench", ) + // If suspended, clean up serving resources but preserve data + if w.Spec.Suspended != nil && *w.Spec.Suspended { + return r.suspendDeployedService(ctx, req, w) + } + // TODO: should do formal spec validation / correction... // check for deprecated databricks location (we did not remove this yet for backwards compat and to allow an upgrade path) @@ -1025,14 +1031,194 @@ func (r *WorkbenchReconciler) CleanupWorkbench(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } +// suspendDeployedService removes serving resources (Deployment, Service, Ingress) +// while preserving data resources (PVC, database, secrets) when Workbench is suspended. +func (r *WorkbenchReconciler) suspendDeployedService(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench) (ctrl.Result, error) { + l := r.GetLogger(ctx).WithValues("event", "suspend-service", "product", "workbench") + + key := client.ObjectKey{Name: w.ComponentName(), Namespace: req.Namespace} + + // INGRESS + if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.Ingress{}); err != nil { + return ctrl.Result{}, err + } + + // SERVICE + if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { + return ctrl.Result{}, err + } + + // DEPLOYMENT + if err := internal.BasicDelete(ctx, r, l, key, &appsv1.Deployment{}); err != nil { + return ctrl.Result{}, err + } + + l.Info("Workbench serving resources suspended") + return ctrl.Result{}, nil +} + func (r *WorkbenchReconciler) cleanupDeployedService(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench) error { l := r.GetLogger(ctx).WithValues( "event", "cleanup-service", - "product", "connect", + "product", "workbench", ) - l.Info("starting") + key := client.ObjectKey{Name: w.ComponentName(), Namespace: req.Namespace} + + // INGRESS + if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.Ingress{}); err != nil { + return err + } + + // SERVICE + if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { + return err + } + + // DEPLOYMENT + if err := internal.BasicDelete(ctx, r, l, key, &appsv1.Deployment{}); err != nil { + return err + } + + // PVCS + // Main volume + if err := internal.BasicDelete(ctx, r, l, key, &corev1.PersistentVolumeClaim{}); err != nil { + return err + } + + // Shared storage PVC (if load balancing is enabled) + sharedStorageKey := client.ObjectKey{ + Name: fmt.Sprintf("%s-shared-storage", w.ComponentName()), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, sharedStorageKey, &corev1.PersistentVolumeClaim{}); err != nil { + return err + } + + // Additional volumes + for _, v := range w.Spec.AdditionalVolumes { + additionalKey := client.ObjectKey{ + Name: v.PvcName, + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, additionalKey, &corev1.PersistentVolumeClaim{}); err != nil { + return err + } + } + + // SERVICE ACCOUNTS + // Main service account (for off-host execution) + if err := internal.BasicDelete(ctx, r, l, key, &corev1.ServiceAccount{}); err != nil { + return err + } + + // Session service account + sessionSaKey := client.ObjectKey{ + Name: w.SessionServiceAccountName(), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, sessionSaKey, &corev1.ServiceAccount{}); err != nil { + return err + } + + // RBAC (Role and RoleBinding for off-host execution) + if err := internal.BasicDelete(ctx, r, l, key, &rbacv1.Role{}); err != nil { + return err + } + + if err := internal.BasicDelete(ctx, r, l, key, &rbacv1.RoleBinding{}); err != nil { + return err + } + + // CONFIGMAPS + if err := internal.BasicDelete(ctx, r, l, key, &corev1.ConfigMap{}); err != nil { + return err + } + + loginCmKey := client.ObjectKey{ + Name: w.LoginConfigmapName(), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, loginCmKey, &corev1.ConfigMap{}); err != nil { + return err + } + + sessionCmKey := client.ObjectKey{ + Name: w.SessionConfigMapName(), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, sessionCmKey, &corev1.ConfigMap{}); err != nil { + return err + } + + supervisorCmKey := client.ObjectKey{ + Name: w.SupervisorConfigmapName(), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, supervisorCmKey, &corev1.ConfigMap{}); err != nil { + return err + } + + templateCmKey := client.ObjectKey{ + Name: w.TemplateConfigMapName(), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, templateCmKey, &corev1.ConfigMap{}); err != nil { + return err + } + + authLoginHtmlCmKey := client.ObjectKey{ + Name: w.AuthLoginPageHtmlConfigmapName(), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, authLoginHtmlCmKey, &corev1.ConfigMap{}); err != nil { + return err + } + + // SECRETS + secretConfigKey := client.ObjectKey{ + Name: fmt.Sprintf("%s-config", w.ComponentName()), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, secretConfigKey, &corev1.Secret{}); err != nil { + return err + } + + // TRAEFIK MIDDLEWARES + cspMiddlewareKey := client.ObjectKey{ + Name: r.CspMiddleware(w), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, cspMiddlewareKey, &v1alpha1.Middleware{}); err != nil { + return err + } + + forwardMiddlewareKey := client.ObjectKey{ + Name: r.ForwardMiddleware(w), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, forwardMiddlewareKey, &v1alpha1.Middleware{}); err != nil { + return err + } + + headersMiddlewareKey := client.ObjectKey{ + Name: r.HeadersMiddleware(w), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, headersMiddlewareKey, &v1alpha1.Middleware{}); err != nil { + return err + } + + // SECRET PROVIDER CLASS + spcKey := client.ObjectKey{ + Name: w.SecretProviderClassName(), + Namespace: req.Namespace, + } + if err := internal.BasicDelete(ctx, r, l, spcKey, &secretstorev1.SecretProviderClass{}); err != nil { + return err + } + l.Info("Workbench service cleanup complete") return nil } From 0eaa2cf19a04dfd4b6bcfcb571592233fb7fc85d Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 13:15:56 -0800 Subject: [PATCH 02/11] fix: address review findings for enable/disable/teardown extension - Remove unused getChronicle test helper - Correct "CRD" to "CR" in cleanup function godoc comments - Fix double blank line in product-team-site-management.md --- docs/guides/product-team-site-management.md | 1 - internal/controller/core/site_controller_chronicle.go | 4 ++-- .../controller/core/site_controller_package_manager.go | 4 ++-- internal/controller/core/site_controller_workbench.go | 4 ++-- internal/controller/core/site_test.go | 7 ------- 5 files changed, 6 insertions(+), 14 deletions(-) diff --git a/docs/guides/product-team-site-management.md b/docs/guides/product-team-site-management.md index 2c8967e..686e937 100644 --- a/docs/guides/product-team-site-management.md +++ b/docs/guides/product-team-site-management.md @@ -468,7 +468,6 @@ spec: # Use teardown: true to permanently delete all Chronicle data. enabled: true - image: "ghcr.io/posit-dev/chronicle:2024.11.0" imagePullPolicy: IfNotPresent diff --git a/internal/controller/core/site_controller_chronicle.go b/internal/controller/core/site_controller_chronicle.go index 09c479f..906a3c5 100644 --- a/internal/controller/core/site_controller_chronicle.go +++ b/internal/controller/core/site_controller_chronicle.go @@ -133,9 +133,9 @@ func (r *SiteReconciler) disableChronicle(ctx context.Context, req controllerrun return nil } -// cleanupChronicle deletes the Chronicle CRD when teardown=true. +// cleanupChronicle deletes the Chronicle CR when teardown=true. // -// WARNING: This is a DESTRUCTIVE operation. Deleting the Chronicle CRD triggers the Chronicle +// WARNING: This is a DESTRUCTIVE operation. Deleting the Chronicle CR triggers the Chronicle // finalizer which permanently destroys: // - All deployed Kubernetes resources // - Chronicle storage (S3 data or local volumes) diff --git a/internal/controller/core/site_controller_package_manager.go b/internal/controller/core/site_controller_package_manager.go index 482b9aa..464a620 100644 --- a/internal/controller/core/site_controller_package_manager.go +++ b/internal/controller/core/site_controller_package_manager.go @@ -165,9 +165,9 @@ func (r *SiteReconciler) disablePackageManager(ctx context.Context, req controll return nil } -// cleanupPackageManager deletes the PackageManager CRD when teardown=true. +// cleanupPackageManager deletes the PackageManager CR when teardown=true. // -// WARNING: This is a DESTRUCTIVE operation. Deleting the PackageManager CRD triggers the PackageManager +// WARNING: This is a DESTRUCTIVE operation. Deleting the PackageManager CR triggers the PackageManager // finalizer which permanently destroys: // - The Package Manager database and all its data // - All secrets (database credentials, provisioning keys, etc.) diff --git a/internal/controller/core/site_controller_workbench.go b/internal/controller/core/site_controller_workbench.go index 8a002d6..69fb297 100644 --- a/internal/controller/core/site_controller_workbench.go +++ b/internal/controller/core/site_controller_workbench.go @@ -568,9 +568,9 @@ func (r *SiteReconciler) disableWorkbench(ctx context.Context, req controllerrun return nil } -// cleanupWorkbench deletes the Workbench CRD when teardown=true. +// cleanupWorkbench deletes the Workbench CR when teardown=true. // -// WARNING: This is a DESTRUCTIVE operation. Deleting the Workbench CRD triggers the Workbench +// WARNING: This is a DESTRUCTIVE operation. Deleting the Workbench CR triggers the Workbench // finalizer which permanently destroys: // - The Workbench database and all its data // - All secrets (database credentials, provisioning keys, etc.) diff --git a/internal/controller/core/site_test.go b/internal/controller/core/site_test.go index 57a989b..232d8f5 100644 --- a/internal/controller/core/site_test.go +++ b/internal/controller/core/site_test.go @@ -323,13 +323,6 @@ func getPackageManager(t *testing.T, cli client.Client, siteNamespace, siteName return pm } -func getChronicle(t *testing.T, cli client.Client, siteNamespace, siteName string) *v1beta1.Chronicle { - chronicle := &v1beta1.Chronicle{} - err := cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle, &client.GetOptions{}) - assert.Nil(t, err) - return chronicle -} - func getFlightdeck(t *testing.T, cli client.Client, siteNamespace, siteName string) *v1beta1.Flightdeck { fd := &v1beta1.Flightdeck{} err := cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, fd, &client.GetOptions{}) From 4755815119eae1130443d2e2ea4697586ac71ef9 Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 13:30:14 -0800 Subject: [PATCH 03/11] chore: sync Helm chart CRDs with config/crd after code generation --- .../crd/core.posit.team_chronicles.yaml | 5 ++ .../crd/core.posit.team_packagemanagers.yaml | 5 ++ .../templates/crd/core.posit.team_sites.yaml | 44 ++++++++++++ .../crd/core.posit.team_workbenches.yaml | 5 ++ internal/controller/core/site_controller.go | 11 ++- .../core/site_controller_chronicle.go | 3 + .../core/site_controller_networkpolicies.go | 6 +- .../core/site_controller_package_manager.go | 3 + .../core/site_controller_workbench.go | 3 + internal/controller/core/site_test.go | 48 +++++++++++++ internal/controller/core/workbench.go | 40 +++++------ internal/controller/core/workbench_test.go | 71 +++++++++++++++++++ 12 files changed, 218 insertions(+), 26 deletions(-) diff --git a/dist/chart/templates/crd/core.posit.team_chronicles.yaml b/dist/chart/templates/crd/core.posit.team_chronicles.yaml index fc41e85..f6da6b4 100755 --- a/dist/chart/templates/crd/core.posit.team_chronicles.yaml +++ b/dist/chart/templates/crd/core.posit.team_chronicles.yaml @@ -139,6 +139,11 @@ spec: additionalProperties: type: string type: object + suspended: + description: |- + Suspended indicates Chronicle should not run serving resources (StatefulSet, Service) + but should preserve configuration. Set by the Site controller. + type: boolean workloadCompoundName: description: WorkloadCompoundName is the name for the workload type: string diff --git a/dist/chart/templates/crd/core.posit.team_packagemanagers.yaml b/dist/chart/templates/crd/core.posit.team_packagemanagers.yaml index 323d55a..ecf607a 100755 --- a/dist/chart/templates/crd/core.posit.team_packagemanagers.yaml +++ b/dist/chart/templates/crd/core.posit.team_packagemanagers.yaml @@ -342,6 +342,11 @@ spec: Sleep puts the service to sleep... so you can debug a crash looping container / etc. It is an ugly escape hatch, but can also be useful on occasion type: boolean + suspended: + description: |- + Suspended indicates Package Manager should not run serving resources (Deployment, Service, Ingress) + but should preserve data resources (PVC, database, secrets). Set by the Site controller. + type: boolean url: type: string volume: diff --git a/dist/chart/templates/crd/core.posit.team_sites.yaml b/dist/chart/templates/crd/core.posit.team_sites.yaml index e0bd60e..2c03c21 100755 --- a/dist/chart/templates/crd/core.posit.team_sites.yaml +++ b/dist/chart/templates/crd/core.posit.team_sites.yaml @@ -73,6 +73,13 @@ spec: type: object agentImage: type: string + enabled: + default: true + description: |- + Enabled controls whether Chronicle is running. Defaults to true. + Setting to false suspends Chronicle: stops the StatefulSet and removes the service. + Re-enabling restores full service. + type: boolean image: type: string imagePullPolicy: @@ -85,6 +92,13 @@ spec: type: object s3Bucket: type: string + teardown: + default: false + description: |- + Teardown permanently destroys all Chronicle resources. + Only takes effect when Enabled is false. + Re-enabling after teardown starts fresh. + type: boolean type: object clusterDate: description: ClusterDate is the date id (YYYYmmdd) for the cluster. @@ -675,6 +689,14 @@ spec: domainPrefix: default: packagemanager type: string + enabled: + default: true + description: |- + Enabled controls whether Package Manager is running. Defaults to true. + Setting to false suspends Package Manager: stops pods and removes ingress/service, + but preserves PVC, database, and secrets so data is retained. + Re-enabling restores full service without data loss. + type: boolean gitSSHKeys: description: |- GitSSHKeys defines SSH key configurations for Git authentication in Package Manager @@ -787,6 +809,13 @@ spec: type: integer s3Bucket: type: string + teardown: + default: false + description: |- + Teardown permanently destroys all Package Manager resources including the database, + secrets, and persistent volume claim. Only takes effect when Enabled is false. + Re-enabling after teardown starts fresh with a new empty database. + type: boolean volume: description: VolumeSpec is a specification for a PersistentVolumeClaim to be created (and/or mounted) @@ -1102,6 +1131,14 @@ spec: domainPrefix: default: workbench type: string + enabled: + default: true + description: |- + Enabled controls whether Workbench is running. Defaults to true. + Setting to false suspends Workbench: stops pods and removes ingress/service, + but preserves PVC, database, and secrets so data is retained. + Re-enabling restores full service without data loss. + type: boolean experimentalFeatures: description: ExperimentalFeatures allows enabling miscellaneous experimental features for workbench @@ -1539,6 +1576,13 @@ spec: clientId: type: string type: object + teardown: + default: false + description: |- + Teardown permanently destroys all Workbench resources including the database, + secrets, and persistent volume claim. Only takes effect when Enabled is false. + Re-enabling after teardown starts fresh with a new empty database. + type: boolean tolerations: description: Tolerations that are applied universally to server and sessions diff --git a/dist/chart/templates/crd/core.posit.team_workbenches.yaml b/dist/chart/templates/crd/core.posit.team_workbenches.yaml index ff0ed92..d850ad1 100755 --- a/dist/chart/templates/crd/core.posit.team_workbenches.yaml +++ b/dist/chart/templates/crd/core.posit.team_workbenches.yaml @@ -7595,6 +7595,11 @@ spec: clientId: type: string type: object + suspended: + description: |- + Suspended indicates Workbench should not run serving resources (Deployment, Service, Ingress) + but should preserve data resources (PVC, database, secrets). Set by the Site controller. + type: boolean tolerations: items: description: |- diff --git a/internal/controller/core/site_controller.go b/internal/controller/core/site_controller.go index bdb9503..471ad6a 100644 --- a/internal/controller/core/site_controller.go +++ b/internal/controller/core/site_controller.go @@ -24,6 +24,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" ) +// isProductEnabled returns true if the product is enabled (nil defaults to enabled). +func isProductEnabled(b *bool) bool { + return b == nil || *b +} + // SiteReconciler reconciles a Site object type SiteReconciler struct { client.Client @@ -162,19 +167,19 @@ func (r *SiteReconciler) reconcileResources(ctx context.Context, req ctrl.Reques l.Info("connect.teardown is set but connect.enabled is not false; teardown has no effect until enabled=false") } - workbenchEnabled := site.Spec.Workbench.Enabled == nil || *site.Spec.Workbench.Enabled + workbenchEnabled := isProductEnabled(site.Spec.Workbench.Enabled) workbenchTeardown := site.Spec.Workbench.Teardown != nil && *site.Spec.Workbench.Teardown if workbenchTeardown && workbenchEnabled { l.Info("workbench.teardown is set but workbench.enabled is not false; teardown has no effect until enabled=false") } - pmEnabled := site.Spec.PackageManager.Enabled == nil || *site.Spec.PackageManager.Enabled + pmEnabled := isProductEnabled(site.Spec.PackageManager.Enabled) pmTeardown := site.Spec.PackageManager.Teardown != nil && *site.Spec.PackageManager.Teardown if pmTeardown && pmEnabled { l.Info("packageManager.teardown is set but packageManager.enabled is not false; teardown has no effect until enabled=false") } - chronicleEnabled := site.Spec.Chronicle.Enabled == nil || *site.Spec.Chronicle.Enabled + chronicleEnabled := isProductEnabled(site.Spec.Chronicle.Enabled) chronicleTeardown := site.Spec.Chronicle.Teardown != nil && *site.Spec.Chronicle.Teardown if chronicleTeardown && chronicleEnabled { l.Info("chronicle.teardown is set but chronicle.enabled is not false; teardown has no effect until enabled=false") diff --git a/internal/controller/core/site_controller_chronicle.go b/internal/controller/core/site_controller_chronicle.go index 906a3c5..e22dafa 100644 --- a/internal/controller/core/site_controller_chronicle.go +++ b/internal/controller/core/site_controller_chronicle.go @@ -43,6 +43,9 @@ func (r *SiteReconciler) reconcileChronicle(ctx context.Context, req controllerr chronicle.Labels = map[string]string{ v1beta1.ManagedByLabelKey: LabelManagedByValue, } + // Suspended is intentionally absent: CreateOrUpdate does a full spec + // replacement (regular Update, not SSA), so any prior Suspended=true is + // cleared when Chronicle is re-enabled. chronicle.Spec = v1beta1.ChronicleSpec{ AwsAccountId: site.Spec.AwsAccountId, ClusterDate: site.Spec.ClusterDate, diff --git a/internal/controller/core/site_controller_networkpolicies.go b/internal/controller/core/site_controller_networkpolicies.go index e5d6ae0..9dedc15 100644 --- a/internal/controller/core/site_controller_networkpolicies.go +++ b/internal/controller/core/site_controller_networkpolicies.go @@ -39,7 +39,7 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. } // Chronicle network policy - chronicleEnabled := site.Spec.Chronicle.Enabled == nil || *site.Spec.Chronicle.Enabled + chronicleEnabled := isProductEnabled(site.Spec.Chronicle.Enabled) if chronicleEnabled { if err := r.reconcileChronicleNetworkPolicy(ctx, req.Namespace, l, site); err != nil { l.Error(err, "error ensuring chronicle network policy") @@ -82,7 +82,7 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. } // Package Manager network policy - pmEnabled := site.Spec.PackageManager.Enabled == nil || *site.Spec.PackageManager.Enabled + pmEnabled := isProductEnabled(site.Spec.PackageManager.Enabled) if pmEnabled { if err := r.reconcilePackageManagerNetworkPolicy(ctx, req.Namespace, l, site); err != nil { l.Error(err, "error ensuring package manager network policy") @@ -96,7 +96,7 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. } // Workbench network policies - workbenchEnabled := site.Spec.Workbench.Enabled == nil || *site.Spec.Workbench.Enabled + workbenchEnabled := isProductEnabled(site.Spec.Workbench.Enabled) if workbenchEnabled { if err := r.reconcileWorkbenchNetworkPolicy(ctx, req.Namespace, l, site); err != nil { l.Error(err, "error ensuring workbench network policy") diff --git a/internal/controller/core/site_controller_package_manager.go b/internal/controller/core/site_controller_package_manager.go index 464a620..6c60176 100644 --- a/internal/controller/core/site_controller_package_manager.go +++ b/internal/controller/core/site_controller_package_manager.go @@ -48,6 +48,9 @@ func (r *SiteReconciler) reconcilePackageManager( pm.Labels = map[string]string{ v1beta1.ManagedByLabelKey: v1beta1.ManagedByLabelValue, } + // Suspended is intentionally absent: CreateOrUpdate does a full spec + // replacement (regular Update, not SSA), so any prior Suspended=true is + // cleared when Package Manager is re-enabled. pm.Spec = v1beta1.PackageManagerSpec{ AwsAccountId: site.Spec.AwsAccountId, ClusterDate: site.Spec.ClusterDate, diff --git a/internal/controller/core/site_controller_workbench.go b/internal/controller/core/site_controller_workbench.go index 69fb297..a77602b 100644 --- a/internal/controller/core/site_controller_workbench.go +++ b/internal/controller/core/site_controller_workbench.go @@ -477,6 +477,9 @@ func (r *SiteReconciler) reconcileWorkbench( if _, err := internal.CreateOrUpdateResource(ctx, r.Client, r.Scheme, l, workbench, site, func() error { workbench.Labels = targetWorkbench.Labels + // Suspended is intentionally absent from targetWorkbench.Spec: CreateOrUpdate + // does a full spec replacement (regular Update, not SSA), so any prior + // Suspended=true is cleared when Workbench is re-enabled. workbench.Spec = targetWorkbench.Spec return nil }); err != nil { diff --git a/internal/controller/core/site_test.go b/internal/controller/core/site_test.go index 232d8f5..2deff50 100644 --- a/internal/controller/core/site_test.go +++ b/internal/controller/core/site_test.go @@ -1534,3 +1534,51 @@ func TestSiteChronicleTeardown(t *testing.T) { err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) assert.Error(t, err, "Chronicle CR should not exist after teardown=true") } + +// TestSiteTeardownIgnoredWhileEnabled verifies that setting teardown=true while a product is +// still enabled (or defaults to enabled) is a no-op: no CRs are deleted. +// This guards the warning-path guard in reconcileResources against accidental removal. +func TestSiteTeardownIgnoredWhileEnabled(t *testing.T) { + siteName := "teardown-while-enabled" + siteNamespace := "posit-team" + + fakeClient := localtest.FakeTestEnv{} + cli, scheme, log := fakeClient.Start(loadSchemes) + rec := SiteReconciler{Client: cli, Scheme: scheme, Log: log} + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: siteNamespace, Name: siteName}} + + // Pass 1: establish running CRs for all three products + site := defaultSite(siteName) + _, err := rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + workbench := &v1beta1.Workbench{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, workbench) + assert.NoError(t, err, "Workbench CR should exist after first reconcile") + + pm := &v1beta1.PackageManager{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, pm) + assert.NoError(t, err, "PackageManager CR should exist after first reconcile") + + chronicle := &v1beta1.Chronicle{} + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) + assert.NoError(t, err, "Chronicle CR should exist after first reconcile") + + // Pass 2: set teardown=true but leave enabled=true (default) — should be a no-op + teardown := true + site.Spec.Workbench.Teardown = &teardown + site.Spec.PackageManager.Teardown = &teardown + site.Spec.Chronicle.Teardown = &teardown + _, err = rec.reconcileResources(context.TODO(), req, site) + assert.NoError(t, err) + + // All CRs should still exist — teardown while enabled is a no-op + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, workbench) + assert.NoError(t, err, "Workbench CR should still exist: teardown has no effect while enabled=true") + + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, pm) + assert.NoError(t, err, "PackageManager CR should still exist: teardown has no effect while enabled=true") + + err = cli.Get(context.TODO(), client.ObjectKey{Name: siteName, Namespace: siteNamespace}, chronicle) + assert.NoError(t, err, "Chronicle CR should still exist: teardown has no effect while enabled=true") +} diff --git a/internal/controller/core/workbench.go b/internal/controller/core/workbench.go index 3065565..f80a167 100644 --- a/internal/controller/core/workbench.go +++ b/internal/controller/core/workbench.go @@ -1031,25 +1031,36 @@ func (r *WorkbenchReconciler) CleanupWorkbench(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } -// suspendDeployedService removes serving resources (Deployment, Service, Ingress) -// while preserving data resources (PVC, database, secrets) when Workbench is suspended. -func (r *WorkbenchReconciler) suspendDeployedService(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench) (ctrl.Result, error) { - l := r.GetLogger(ctx).WithValues("event", "suspend-service", "product", "workbench") - +// deleteServingResources removes Ingress, Service, and Deployment for Workbench. +// Called by both suspendDeployedService (data preserved) and cleanupDeployedService (full teardown). +func (r *WorkbenchReconciler) deleteServingResources(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench) error { + l := r.GetLogger(ctx).WithValues("product", "workbench") key := client.ObjectKey{Name: w.ComponentName(), Namespace: req.Namespace} // INGRESS if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.Ingress{}); err != nil { - return ctrl.Result{}, err + return err } // SERVICE if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { - return ctrl.Result{}, err + return err } // DEPLOYMENT if err := internal.BasicDelete(ctx, r, l, key, &appsv1.Deployment{}); err != nil { + return err + } + + return nil +} + +// suspendDeployedService removes serving resources (Deployment, Service, Ingress) +// while preserving data resources (PVC, database, secrets) when Workbench is suspended. +func (r *WorkbenchReconciler) suspendDeployedService(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench) (ctrl.Result, error) { + l := r.GetLogger(ctx).WithValues("event", "suspend-service", "product", "workbench") + + if err := r.deleteServingResources(ctx, req, w); err != nil { return ctrl.Result{}, err } @@ -1063,22 +1074,11 @@ func (r *WorkbenchReconciler) cleanupDeployedService(ctx context.Context, req ct "product", "workbench", ) - key := client.ObjectKey{Name: w.ComponentName(), Namespace: req.Namespace} - - // INGRESS - if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.Ingress{}); err != nil { + if err := r.deleteServingResources(ctx, req, w); err != nil { return err } - // SERVICE - if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { - return err - } - - // DEPLOYMENT - if err := internal.BasicDelete(ctx, r, l, key, &appsv1.Deployment{}); err != nil { - return err - } + key := client.ObjectKey{Name: w.ComponentName(), Namespace: req.Namespace} // PVCS // Main volume diff --git a/internal/controller/core/workbench_test.go b/internal/controller/core/workbench_test.go index ea49444..3fc185e 100644 --- a/internal/controller/core/workbench_test.go +++ b/internal/controller/core/workbench_test.go @@ -11,6 +11,7 @@ import ( "github.com/posit-dev/team-operator/internal" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -389,3 +390,73 @@ func TestWorkbenchPodDisruptionBudgets(t *testing.T) { assert.Equal(t, int32(0), sessionPdb.Spec.MaxUnavailable.IntVal, "Session PDB should have maxUnavailable=0 to prevent session evictions") } + +// TestWorkbenchReconciler_Suspended verifies that when Workbench has Suspended=true, +// ReconcileWorkbench does not create serving resources (Deployment, Service, Ingress). +func TestWorkbenchReconciler_Suspended(t *testing.T) { + ctx := context.Background() + ns := "posit-team" + name := "workbench-suspended" + + ctx, r, req, cli := initWorkbenchReconciler(t, ctx, ns, name) + + wb := defineDefaultWorkbench(t, ns, name) + suspended := true + wb.Spec.Suspended = &suspended + + err := internal.BasicCreateOrUpdate(ctx, r, r.GetLogger(ctx), req.NamespacedName, &positcov1beta1.Workbench{}, wb) + require.NoError(t, err) + + wb = getWorkbench(t, cli, ns, name) + + res, err := r.ReconcileWorkbench(ctx, req, wb) + require.NoError(t, err) + require.True(t, res.IsZero()) + + // No Deployment should be created when suspended + dep := &appsv1.Deployment{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, dep) + assert.Error(t, err, "Deployment should not exist when Workbench is suspended") +} + +// TestWorkbenchReconciler_SuspendRemovesDeployment verifies that when Workbench transitions +// to Suspended=true, the Deployment is removed while data resources are preserved. +func TestWorkbenchReconciler_SuspendRemovesDeployment(t *testing.T) { + ctx := context.Background() + ns := "posit-team" + name := "workbench-suspend-removes" + + ctx, r, req, cli := initWorkbenchReconciler(t, ctx, ns, name) + + wb := defineDefaultWorkbench(t, ns, name) + + err := internal.BasicCreateOrUpdate(ctx, r, r.GetLogger(ctx), req.NamespacedName, &positcov1beta1.Workbench{}, wb) + require.NoError(t, err) + + wb = getWorkbench(t, cli, ns, name) + + // Pass 1: normal reconcile — Deployment should be created + res, err := r.ReconcileWorkbench(ctx, req, wb) + require.NoError(t, err) + require.True(t, res.IsZero()) + + dep := &appsv1.Deployment{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, dep) + require.NoError(t, err, "Deployment should exist after normal reconcile") + + // Pass 2: suspend — Deployment should be removed + wb = getWorkbench(t, cli, ns, name) + suspended := true + wb.Spec.Suspended = &suspended + err = cli.Update(ctx, wb) + require.NoError(t, err) + + wb = getWorkbench(t, cli, ns, name) + res, err = r.ReconcileWorkbench(ctx, req, wb) + require.NoError(t, err) + require.True(t, res.IsZero()) + + dep = &appsv1.Deployment{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, dep) + assert.Error(t, err, "Deployment should be removed when Workbench is suspended") +} From acb367ca2a01885e50a05668be19da16df592211 Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 13:30:14 -0800 Subject: [PATCH 04/11] Address review findings (job 94) Changes: - Pass caller's logger into `deleteServingResources` so `BasicDelete` log lines carry the `event` key (`suspend-service` or `cleanup-service`) from the calling function - Add `logr.Logger` parameter to `deleteServingResources` signature and remove its internal logger construction - Update `suspendDeployedService` and `cleanupDeployedService` to pass their `l` to `deleteServingResources` - Add `networkingv1` import to `workbench_test.go` - Extend `TestWorkbenchReconciler_Suspended` to assert `Service` and `Ingress` are also absent when suspended - Extend `TestWorkbenchReconciler_SuspendRemovesDeployment` to assert `Service` and `Ingress` are also removed on suspend --- internal/controller/core/workbench.go | 8 ++++---- internal/controller/core/workbench_test.go | 19 ++++++++++++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/internal/controller/core/workbench.go b/internal/controller/core/workbench.go index f80a167..cc86a88 100644 --- a/internal/controller/core/workbench.go +++ b/internal/controller/core/workbench.go @@ -6,6 +6,7 @@ import ( "regexp" "strconv" + "github.com/go-logr/logr" "github.com/pkg/errors" positcov1beta1 "github.com/posit-dev/team-operator/api/core/v1beta1" "github.com/posit-dev/team-operator/api/product" @@ -1033,8 +1034,7 @@ func (r *WorkbenchReconciler) CleanupWorkbench(ctx context.Context, req ctrl.Req // deleteServingResources removes Ingress, Service, and Deployment for Workbench. // Called by both suspendDeployedService (data preserved) and cleanupDeployedService (full teardown). -func (r *WorkbenchReconciler) deleteServingResources(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench) error { - l := r.GetLogger(ctx).WithValues("product", "workbench") +func (r *WorkbenchReconciler) deleteServingResources(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench, l logr.Logger) error { key := client.ObjectKey{Name: w.ComponentName(), Namespace: req.Namespace} // INGRESS @@ -1060,7 +1060,7 @@ func (r *WorkbenchReconciler) deleteServingResources(ctx context.Context, req ct func (r *WorkbenchReconciler) suspendDeployedService(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench) (ctrl.Result, error) { l := r.GetLogger(ctx).WithValues("event", "suspend-service", "product", "workbench") - if err := r.deleteServingResources(ctx, req, w); err != nil { + if err := r.deleteServingResources(ctx, req, w, l); err != nil { return ctrl.Result{}, err } @@ -1074,7 +1074,7 @@ func (r *WorkbenchReconciler) cleanupDeployedService(ctx context.Context, req ct "product", "workbench", ) - if err := r.deleteServingResources(ctx, req, w); err != nil { + if err := r.deleteServingResources(ctx, req, w, l); err != nil { return err } diff --git a/internal/controller/core/workbench_test.go b/internal/controller/core/workbench_test.go index 3fc185e..4a27e21 100644 --- a/internal/controller/core/workbench_test.go +++ b/internal/controller/core/workbench_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -413,10 +414,18 @@ func TestWorkbenchReconciler_Suspended(t *testing.T) { require.NoError(t, err) require.True(t, res.IsZero()) - // No Deployment should be created when suspended + // No serving resources should be created when suspended dep := &appsv1.Deployment{} err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, dep) assert.Error(t, err, "Deployment should not exist when Workbench is suspended") + + svc := &corev1.Service{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, svc) + assert.Error(t, err, "Service should not exist when Workbench is suspended") + + ing := &networkingv1.Ingress{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, ing) + assert.Error(t, err, "Ingress should not exist when Workbench is suspended") } // TestWorkbenchReconciler_SuspendRemovesDeployment verifies that when Workbench transitions @@ -459,4 +468,12 @@ func TestWorkbenchReconciler_SuspendRemovesDeployment(t *testing.T) { dep = &appsv1.Deployment{} err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, dep) assert.Error(t, err, "Deployment should be removed when Workbench is suspended") + + svc := &corev1.Service{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, svc) + assert.Error(t, err, "Service should be removed when Workbench is suspended") + + ing := &networkingv1.Ingress{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, ing) + assert.Error(t, err, "Ingress should be removed when Workbench is suspended") } From aa9d194df53ad70affa4ee6ee35b30f9c25c5452 Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 13:30:14 -0800 Subject: [PATCH 05/11] Address review findings (job 98) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes: - `chronicle_controller.go`: Added comment in `CleanupChronicle` noting that PVC cleanup would be needed if `VolumeClaimTemplates` are ever added (currently Chronicle uses EmptyDir/S3, so no PVCs exist) - `workbench.go`: Moved `db.CleanupDatabasePasswordSecret` call into `cleanupDeployedService` (under the SECRETS section) and removed it from `CleanupWorkbench` to avoid redundancy — ensures the DB credential secret is deleted in the full teardown path - `site_controller_chronicle.go`: Fixed misleading doc comment on `cleanupChronicle` — removed false claim that S3 data is destroyed; clarified that only Kubernetes resources and local EmptyDir volumes are deleted, and S3 buckets must be cleaned up separately - `workbench_test.go`: Added assertion in `TestWorkbenchReconciler_SuspendRemovesDeployment` that the login ConfigMap (`-login`) is preserved after suspension, verifying the "data resources are not deleted" promise --- internal/controller/core/chronicle_controller.go | 3 +++ internal/controller/core/site_controller_chronicle.go | 5 +++-- internal/controller/core/workbench.go | 8 +++++--- internal/controller/core/workbench_test.go | 5 +++++ 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/internal/controller/core/chronicle_controller.go b/internal/controller/core/chronicle_controller.go index 615f9da..72371f4 100644 --- a/internal/controller/core/chronicle_controller.go +++ b/internal/controller/core/chronicle_controller.go @@ -349,6 +349,9 @@ func (r *ChronicleReconciler) CleanupChronicle(ctx context.Context, req ctrl.Req if err := internal.BasicDelete(ctx, r, l, key, &v1.StatefulSet{}); err != nil { return ctrl.Result{}, err } + // NOTE: Chronicle's StatefulSet does not use VolumeClaimTemplates (it uses EmptyDir or S3). + // If VolumeClaimTemplates are added in the future, their PVCs must be deleted explicitly + // here because Kubernetes does not garbage-collect StatefulSet PVCs automatically. // CONFIGMAP if err := internal.BasicDelete(ctx, r, l, key, &corev1.ConfigMap{}); err != nil { diff --git a/internal/controller/core/site_controller_chronicle.go b/internal/controller/core/site_controller_chronicle.go index e22dafa..2844131 100644 --- a/internal/controller/core/site_controller_chronicle.go +++ b/internal/controller/core/site_controller_chronicle.go @@ -141,9 +141,10 @@ func (r *SiteReconciler) disableChronicle(ctx context.Context, req controllerrun // WARNING: This is a DESTRUCTIVE operation. Deleting the Chronicle CR triggers the Chronicle // finalizer which permanently destroys: // - All deployed Kubernetes resources -// - Chronicle storage (S3 data or local volumes) +// - Local EmptyDir volumes (if configured) // -// Note: Chronicle does not use a database or persistent volumes like other products. +// Note: S3 data is NOT deleted by the operator; the S3 bucket must be cleaned up separately. +// Chronicle does not use a database or persistent volumes like other products. // // This is triggered by Site.Spec.Chronicle.Teardown=true (when Enabled=false). // Re-enabling Chronicle after teardown will start fresh. diff --git a/internal/controller/core/workbench.go b/internal/controller/core/workbench.go index cc86a88..01f7f6a 100644 --- a/internal/controller/core/workbench.go +++ b/internal/controller/core/workbench.go @@ -1023,9 +1023,6 @@ func (r *WorkbenchReconciler) CleanupWorkbench(ctx context.Context, req ctrl.Req if err := r.cleanupDeployedService(ctx, req, w); err != nil { return ctrl.Result{}, err } - if err := db.CleanupDatabasePasswordSecret(ctx, r, req, w.ComponentName()); err != nil { - return ctrl.Result{}, err - } if err := db.CleanupDatabase(ctx, r, req, w.ComponentName()); err != nil { return ctrl.Result{}, err } @@ -1184,6 +1181,11 @@ func (r *WorkbenchReconciler) cleanupDeployedService(ctx context.Context, req ct return err } + // Database password secret (created by EnsureDatabaseExists) + if err := db.CleanupDatabasePasswordSecret(ctx, r, req, w.ComponentName()); err != nil { + return err + } + // TRAEFIK MIDDLEWARES cspMiddlewareKey := client.ObjectKey{ Name: r.CspMiddleware(w), diff --git a/internal/controller/core/workbench_test.go b/internal/controller/core/workbench_test.go index 4a27e21..0512820 100644 --- a/internal/controller/core/workbench_test.go +++ b/internal/controller/core/workbench_test.go @@ -476,4 +476,9 @@ func TestWorkbenchReconciler_SuspendRemovesDeployment(t *testing.T) { ing := &networkingv1.Ingress{} err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, ing) assert.Error(t, err, "Ingress should be removed when Workbench is suspended") + + // Data resources must be preserved during suspension + loginCm := &corev1.ConfigMap{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.LoginConfigmapName(), Namespace: ns}, loginCm) + assert.NoError(t, err, "Login ConfigMap should be preserved when Workbench is suspended") } From eb7848f220308e6a2a11a8876ae693e39dc773c6 Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 13:30:14 -0800 Subject: [PATCH 06/11] Address review findings (job 105) Build passes. Tests fail only due to missing `etcd` in the sandbox, which affects all tests in the package equally (pre-existing environment constraint). Changes: - Add `TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret`: pre-creates the DB password secret and asserts it is deleted after `CleanupWorkbench` runs - Add DB password secret pre-creation and preservation assertion to `TestWorkbenchReconciler_SuspendRemovesDeployment`: verifies the secret is not deleted when Workbench is suspended --- internal/controller/core/workbench_test.go | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/internal/controller/core/workbench_test.go b/internal/controller/core/workbench_test.go index 0512820..5dc1260 100644 --- a/internal/controller/core/workbench_test.go +++ b/internal/controller/core/workbench_test.go @@ -453,6 +453,16 @@ func TestWorkbenchReconciler_SuspendRemovesDeployment(t *testing.T) { err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, dep) require.NoError(t, err, "Deployment should exist after normal reconcile") + // Pre-create DB password secret to verify it is preserved during suspension + pwSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: wb.ComponentName(), + Namespace: ns, + }, + } + err = cli.Create(ctx, pwSecret) + require.NoError(t, err) + // Pass 2: suspend — Deployment should be removed wb = getWorkbench(t, cli, ns, name) suspended := true @@ -481,4 +491,49 @@ func TestWorkbenchReconciler_SuspendRemovesDeployment(t *testing.T) { loginCm := &corev1.ConfigMap{} err = cli.Get(ctx, client.ObjectKey{Name: wb.LoginConfigmapName(), Namespace: ns}, loginCm) assert.NoError(t, err, "Login ConfigMap should be preserved when Workbench is suspended") + + // DB password secret must also be preserved during suspension + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, pwSecret) + assert.NoError(t, err, "DB password secret should be preserved when Workbench is suspended") +} + +// TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret verifies that CleanupWorkbench +// deletes the DB password secret. +func TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret(t *testing.T) { + ctx := context.Background() + ns := "posit-team" + name := "workbench-cleanup-db-secret" + + ctx, r, req, cli := initWorkbenchReconciler(t, ctx, ns, name) + + wb := defineDefaultWorkbench(t, ns, name) + + err := internal.BasicCreateOrUpdate(ctx, r, r.GetLogger(ctx), req.NamespacedName, &positcov1beta1.Workbench{}, wb) + require.NoError(t, err) + + wb = getWorkbench(t, cli, ns, name) + + // Pre-create the DB password secret + pwSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: wb.ComponentName(), + Namespace: ns, + }, + } + err = cli.Create(ctx, pwSecret) + require.NoError(t, err) + + // Verify it exists before cleanup + existing := &corev1.Secret{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, existing) + require.NoError(t, err, "DB password secret should exist before cleanup") + + // Run CleanupWorkbench + _, err = r.CleanupWorkbench(ctx, req, wb) + require.NoError(t, err) + + // Assert the secret is gone + deleted := &corev1.Secret{} + err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, deleted) + assert.Error(t, err, "DB password secret should be deleted after CleanupWorkbench") } From 417fcf5e8916f38f931772e9737bd6d79e41ed4e Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 13:30:14 -0800 Subject: [PATCH 07/11] Address review findings (job 109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test failures are all due to `etcd` not being available in this sandbox environment — these integration tests require `kubebuilder` control plane binaries. The failures pre-exist and are unrelated to the changes. The build compiles cleanly. Changes: - Add `apierrors "k8s.io/apimachinery/pkg/api/errors"` import to `workbench_test.go` - Replace `assert.Error(t, err, ...)` with `assert.True(t, apierrors.IsNotFound(err), ...)` in `TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret` for a precise not-found assertion --- internal/controller/core/workbench_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/controller/core/workbench_test.go b/internal/controller/core/workbench_test.go index 5dc1260..a150148 100644 --- a/internal/controller/core/workbench_test.go +++ b/internal/controller/core/workbench_test.go @@ -14,6 +14,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -535,5 +536,5 @@ func TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret(t *testing.T) // Assert the secret is gone deleted := &corev1.Secret{} err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, deleted) - assert.Error(t, err, "DB password secret should be deleted after CleanupWorkbench") + assert.True(t, apierrors.IsNotFound(err), "DB password secret should be deleted after CleanupWorkbench") } From 669c7fc4ff09af5df2432ea5a5dcbb8ed93158f6 Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 13:30:14 -0800 Subject: [PATCH 08/11] Address review findings (job 110) Everything looks correct. Build passes, changes are consistent. Here's the summary: --- Changes: - Added `db.PasswordSecretName(name string) string` function returning `name + "-db-password"`, following the codebase's naming convention (e.g. `KeySecretName`, `LoginConfigmapName`) - Updated `CleanupDatabasePasswordSecret` to look up/delete the secret at `PasswordSecretName(name)` instead of the raw component name - Updated `EnsureDatabasePasswordSecretAndReturnIt` to create the secret at `PasswordSecretName(name)` for consistency - Updated `TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret` to create and assert at `db.PasswordSecretName(wb.ComponentName())`, ensuring the test name aligns with what `CleanupWorkbench` actually deletes (non-vacuous test) - Updated `TestWorkbenchReconciler_Reconcile_SuspendResume` to create and check preservation at `db.PasswordSecretName(wb.ComponentName())` - Added `db` package import to `workbench_test.go` --- internal/controller/core/workbench_test.go | 12 +++++++----- internal/db/db.go | 11 +++++++++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/internal/controller/core/workbench_test.go b/internal/controller/core/workbench_test.go index a150148..c6d7e3a 100644 --- a/internal/controller/core/workbench_test.go +++ b/internal/controller/core/workbench_test.go @@ -9,6 +9,7 @@ import ( localtest "github.com/posit-dev/team-operator/api/localtest" "github.com/posit-dev/team-operator/api/product" "github.com/posit-dev/team-operator/internal" + "github.com/posit-dev/team-operator/internal/db" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" @@ -457,7 +458,7 @@ func TestWorkbenchReconciler_SuspendRemovesDeployment(t *testing.T) { // Pre-create DB password secret to verify it is preserved during suspension pwSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: wb.ComponentName(), + Name: db.PasswordSecretName(wb.ComponentName()), Namespace: ns, }, } @@ -494,7 +495,7 @@ func TestWorkbenchReconciler_SuspendRemovesDeployment(t *testing.T) { assert.NoError(t, err, "Login ConfigMap should be preserved when Workbench is suspended") // DB password secret must also be preserved during suspension - err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, pwSecret) + err = cli.Get(ctx, client.ObjectKey{Name: db.PasswordSecretName(wb.ComponentName()), Namespace: ns}, pwSecret) assert.NoError(t, err, "DB password secret should be preserved when Workbench is suspended") } @@ -515,9 +516,10 @@ func TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret(t *testing.T) wb = getWorkbench(t, cli, ns, name) // Pre-create the DB password secret + secretName := db.PasswordSecretName(wb.ComponentName()) pwSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: wb.ComponentName(), + Name: secretName, Namespace: ns, }, } @@ -526,7 +528,7 @@ func TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret(t *testing.T) // Verify it exists before cleanup existing := &corev1.Secret{} - err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, existing) + err = cli.Get(ctx, client.ObjectKey{Name: secretName, Namespace: ns}, existing) require.NoError(t, err, "DB password secret should exist before cleanup") // Run CleanupWorkbench @@ -535,6 +537,6 @@ func TestWorkbenchReconciler_CleanupDeletesDatabasePasswordSecret(t *testing.T) // Assert the secret is gone deleted := &corev1.Secret{} - err = cli.Get(ctx, client.ObjectKey{Name: wb.ComponentName(), Namespace: ns}, deleted) + err = cli.Get(ctx, client.ObjectKey{Name: secretName, Namespace: ns}, deleted) assert.True(t, apierrors.IsNotFound(err), "DB password secret should be deleted after CleanupWorkbench") } diff --git a/internal/db/db.go b/internal/db/db.go index 7e5ce11..b75bc3d 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -179,7 +179,14 @@ func EnsureDatabaseExists( return nil } +// PasswordSecretName returns the Kubernetes Secret name used to store the database password +// for a component identified by name. +func PasswordSecretName(name string) string { + return fmt.Sprintf("%s-db-password", name) +} + func CleanupDatabasePasswordSecret(ctx context.Context, r product.SomeReconciler, req ctrl.Request, name string) error { + secretName := PasswordSecretName(name) l := r.GetLogger(ctx).WithValues( "event", "cleanup-database-password-secret", "database", name, @@ -190,7 +197,7 @@ func CleanupDatabasePasswordSecret(ctx context.Context, r product.SomeReconciler // Kubernetes s := &corev1.Secret{} - if err := r.Get(ctx, client.ObjectKey{Name: name, Namespace: req.Namespace}, s); err != nil && apierrors.IsNotFound(err) { + if err := r.Get(ctx, client.ObjectKey{Name: secretName, Namespace: req.Namespace}, s); err != nil && apierrors.IsNotFound(err) { // secret is missing, move on and clean up any other secrets } else if err != nil { l.Error(err, "error trying to get kubernetes secret before deletion") @@ -229,7 +236,7 @@ func EnsureDatabasePasswordSecretAndReturnIt(ctx context.Context, r product.Some switch secretType { case product.SiteSecretKubernetes: secretData, err := internal.EnsureSecretKubernetes( - ctx, r, req, owner, name, generatePasswordSecretData, + ctx, r, req, owner, PasswordSecretName(name), generatePasswordSecretData, ) if err != nil { l.Error(err, "error creating secret in kubernetes") From ad4128015ff539de153332350f265221c9a218fe Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 23 Feb 2026 13:30:14 -0800 Subject: [PATCH 09/11] Address review findings (job 117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build succeeds. The test failures are all due to a missing etcd binary (`/usr/local/kubebuilder/bin/etcd: no such file or directory`) — this is a pre-existing environment issue with kubebuilder's envtest infrastructure, unrelated to my change (which only modified documentation). Changes: - Add v1.15.0 upgrade notes to `docs/guides/upgrading.md` documenting the DB password secret rename from `` to `-db-password`, with step-by-step migration instructions for existing deployments --- docs/guides/upgrading.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/docs/guides/upgrading.md b/docs/guides/upgrading.md index 1ccb73e..e3a3c12 100644 --- a/docs/guides/upgrading.md +++ b/docs/guides/upgrading.md @@ -161,6 +161,43 @@ kubectl get sites -A -o json | jq '.items[] | select(.status.conditions[]?.reaso ## Version-Specific Migrations +### v1.15.0 + +**Breaking Change: Database Password Secret Rename** + +The Kubernetes Secret used to store the database password for each product component has been renamed from `` to `-db-password`. + +If you are upgrading an existing installation that has already run the operator against live clusters, you must migrate the existing secrets before upgrading. Otherwise, the operator will create new secrets at the new name with freshly generated passwords, leaving the old secrets orphaned and causing database authentication failures. + +**Migration steps (run before upgrading the operator):** + +1. Identify the components with existing DB password secrets: + + ```bash + kubectl get secrets -n posit-team -o name | grep -v db-password + ``` + +2. For each component (workbench, connect, packagemanager), rename the secret: + + ```bash + # Get the old secret data + OLD_NAME= + NEW_NAME="${OLD_NAME}-db-password" + NAMESPACE=posit-team + + # Create new secret with old data + kubectl get secret "${OLD_NAME}" -n "${NAMESPACE}" -o json \ + | python3 -c "import json,sys; d=json.load(sys.stdin); d['metadata']['name']='${NEW_NAME}'; [d['metadata'].pop(k,None) for k in ['resourceVersion','uid','creationTimestamp','ownerReferences']]; print(json.dumps(d))" \ + | kubectl apply -f - + + # Delete old secret + kubectl delete secret "${OLD_NAME}" -n "${NAMESPACE}" + ``` + +3. Proceed with the operator upgrade. + +If you are performing a fresh installation or upgrading a cluster that has never had the operator running against it, no migration is needed. + ### v1.2.0 **New Features:** From 2dcb600c55c3e6995f1ccb324d8d104067e5904a Mon Sep 17 00:00:00 2001 From: ian-flores Date: Thu, 26 Feb 2026 17:05:19 -0800 Subject: [PATCH 10/11] fix: address Lytol review nits - Add BatchDelete helper to internal; use in Chronicle, Connect, Package Manager, and Workbench cleanup/suspend functions - Rename isProductEnabled to checkBool(ptr *bool, defaultVal bool) and apply consistently for both enabled and teardown patterns across site_controller.go and site_controller_networkpolicies.go --- docs/guides/upgrading.md | 8 ++- .../controller/core/chronicle_controller.go | 34 +++------- internal/controller/core/connect.go | 59 ++++------------- internal/controller/core/package_manager.go | 64 ++++--------------- internal/controller/core/site_controller.go | 25 ++++---- .../core/site_controller_networkpolicies.go | 8 +-- internal/controller/core/workbench.go | 21 ++---- internal/kubernetes_crudding.go | 10 +++ 8 files changed, 75 insertions(+), 154 deletions(-) diff --git a/docs/guides/upgrading.md b/docs/guides/upgrading.md index e3a3c12..4b9e23c 100644 --- a/docs/guides/upgrading.md +++ b/docs/guides/upgrading.md @@ -174,11 +174,15 @@ If you are upgrading an existing installation that has already run the operator 1. Identify the components with existing DB password secrets: ```bash - kubectl get secrets -n posit-team -o name | grep -v db-password + for comp in workbench connect packagemanager; do + kubectl get secret "${comp}" -n posit-team --ignore-not-found -o name + done ``` 2. For each component (workbench, connect, packagemanager), rename the secret: + > **Warning:** If `${NEW_NAME}` already exists in the cluster, do not apply this migration — the operator has already generated a new password and you must re-synchronize the database password manually. + ```bash # Get the old secret data OLD_NAME= @@ -187,7 +191,7 @@ If you are upgrading an existing installation that has already run the operator # Create new secret with old data kubectl get secret "${OLD_NAME}" -n "${NAMESPACE}" -o json \ - | python3 -c "import json,sys; d=json.load(sys.stdin); d['metadata']['name']='${NEW_NAME}'; [d['metadata'].pop(k,None) for k in ['resourceVersion','uid','creationTimestamp','ownerReferences']]; print(json.dumps(d))" \ + | python3 -c "import json,sys; d=json.load(sys.stdin); d['metadata']['name']='${NEW_NAME}'; [d['metadata'].pop(k,None) for k in ['resourceVersion','uid','creationTimestamp','managedFields','ownerReferences']]; print(json.dumps(d))" \ | kubectl apply -f - # Delete old secret diff --git a/internal/controller/core/chronicle_controller.go b/internal/controller/core/chronicle_controller.go index 72371f4..82e609e 100644 --- a/internal/controller/core/chronicle_controller.go +++ b/internal/controller/core/chronicle_controller.go @@ -340,26 +340,15 @@ func (r *ChronicleReconciler) CleanupChronicle(ctx context.Context, req ctrl.Req key := client.ObjectKey{Name: c.ComponentName(), Namespace: req.Namespace} - // SERVICE - if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { - return ctrl.Result{}, err - } - - // STATEFULSET - if err := internal.BasicDelete(ctx, r, l, key, &v1.StatefulSet{}); err != nil { - return ctrl.Result{}, err - } // NOTE: Chronicle's StatefulSet does not use VolumeClaimTemplates (it uses EmptyDir or S3). // If VolumeClaimTemplates are added in the future, their PVCs must be deleted explicitly // here because Kubernetes does not garbage-collect StatefulSet PVCs automatically. - - // CONFIGMAP - if err := internal.BasicDelete(ctx, r, l, key, &corev1.ConfigMap{}); err != nil { - return ctrl.Result{}, err - } - - // SERVICE ACCOUNTS - if err := internal.BasicDelete(ctx, r, l, key, &corev1.ServiceAccount{}); err != nil { + if err := internal.BatchDelete(ctx, r, l, key, + &corev1.Service{}, + &v1.StatefulSet{}, + &corev1.ConfigMap{}, + &corev1.ServiceAccount{}, + ); err != nil { return ctrl.Result{}, err } @@ -383,13 +372,10 @@ func (r *ChronicleReconciler) suspendDeployedService(ctx context.Context, req ct key := client.ObjectKey{Name: c.ComponentName(), Namespace: req.Namespace} - // SERVICE - if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { - return ctrl.Result{}, err - } - - // STATEFULSET (Chronicle uses StatefulSet, not Deployment) - if err := internal.BasicDelete(ctx, r, l, key, &v1.StatefulSet{}); err != nil { + if err := internal.BatchDelete(ctx, r, l, key, + &corev1.Service{}, + &v1.StatefulSet{}, // Chronicle uses StatefulSet, not Deployment + ); err != nil { return ctrl.Result{}, err } diff --git a/internal/controller/core/connect.go b/internal/controller/core/connect.go index ec50761..7beca1f 100644 --- a/internal/controller/core/connect.go +++ b/internal/controller/core/connect.go @@ -886,13 +886,11 @@ func (r *ConnectReconciler) suspendDeployedService(ctx context.Context, req ctrl key := client.ObjectKey{Name: c.ComponentName(), Namespace: req.Namespace} - if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.Ingress{}); err != nil { - return ctrl.Result{}, err - } - if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { - return ctrl.Result{}, err - } - if err := internal.BasicDelete(ctx, r, l, key, &v1.Deployment{}); err != nil { + if err := internal.BatchDelete(ctx, r, l, key, + &networkingv1.Ingress{}, + &corev1.Service{}, + &v1.Deployment{}, + ); err != nil { return ctrl.Result{}, err } @@ -912,48 +910,17 @@ func (r *ConnectReconciler) cleanupDeployedService(ctx context.Context, req ctrl Namespace: req.Namespace, } - // INGRESS - - existingIngress := &networkingv1.Ingress{} - if err := internal.BasicDelete(ctx, r, l, key, existingIngress); err != nil { - return err - } - - // SERVICE - - existingService := &corev1.Service{} - if err := internal.BasicDelete(ctx, r, l, key, existingService); err != nil { - return err - } - - // DEPLOYMENT - - existingDeployment := &v1.Deployment{} - if err := internal.BasicDelete(ctx, r, l, key, existingDeployment); err != nil { - return err - } - - // VOLUME // NOTE: we delete the volume universally, even if create was false... // this ensures that we have the resource completely removed, whether it // was created, forgotten, or never created. - - existingPvc := &corev1.PersistentVolumeClaim{} - if err := internal.BasicDelete(ctx, r, l, key, existingPvc); err != nil { - return err - } - - // SERVICE ACCOUNT - - existingServiceAccount := &corev1.ServiceAccount{} - if err := internal.BasicDelete(ctx, r, l, key, existingServiceAccount); err != nil { - return err - } - - // CONFIGMAP - - existingConfigmap := &corev1.ConfigMap{} - if err := internal.BasicDelete(ctx, r, l, key, existingConfigmap); err != nil { + if err := internal.BatchDelete(ctx, r, l, key, + &networkingv1.Ingress{}, + &corev1.Service{}, + &v1.Deployment{}, + &corev1.PersistentVolumeClaim{}, + &corev1.ServiceAccount{}, + &corev1.ConfigMap{}, + ); err != nil { return err } diff --git a/internal/controller/core/package_manager.go b/internal/controller/core/package_manager.go index 2cbf1b5..e3db0f9 100644 --- a/internal/controller/core/package_manager.go +++ b/internal/controller/core/package_manager.go @@ -52,18 +52,11 @@ func (r *PackageManagerReconciler) suspendDeployedService(ctx context.Context, r key := client.ObjectKey{Name: pm.ComponentName(), Namespace: req.Namespace} - // INGRESS - if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.Ingress{}); err != nil { - return ctrl.Result{}, err - } - - // SERVICE - if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { - return ctrl.Result{}, err - } - - // DEPLOYMENT - if err := internal.BasicDelete(ctx, r, l, key, &v1.Deployment{}); err != nil { + if err := internal.BatchDelete(ctx, r, l, key, + &networkingv1.Ingress{}, + &corev1.Service{}, + &v1.Deployment{}, + ); err != nil { return ctrl.Result{}, err } @@ -83,48 +76,17 @@ func (r *PackageManagerReconciler) cleanupDeployedService(ctx context.Context, r Namespace: req.Namespace, } - // INGRESS - - existingIngress := &networkingv1.Ingress{} - if err := internal.BasicDelete(ctx, r, l, key, existingIngress); err != nil { - return err - } - - // SERVICE - - existingService := &corev1.Service{} - if err := internal.BasicDelete(ctx, r, l, key, existingService); err != nil { - return err - } - - // DEPLOYMENT - - existingDeployment := &v1.Deployment{} - if err := internal.BasicDelete(ctx, r, l, key, existingDeployment); err != nil { - return err - } - - // VOLUME // NOTE: we delete the volume universally, even if create was false... // this ensures that we have the resource completely removed, whether it // was created, forgotten, or never created. - - existingPvc := &corev1.PersistentVolumeClaim{} - if err := internal.BasicDelete(ctx, r, l, key, existingPvc); err != nil { - return err - } - - // SERVICE ACCOUNT - - existingServiceAccount := &corev1.ServiceAccount{} - if err := internal.BasicDelete(ctx, r, l, key, existingServiceAccount); err != nil { - return err - } - - // CONFIGMAP - - existingConfigmap := &corev1.ConfigMap{} - if err := internal.BasicDelete(ctx, r, l, key, existingConfigmap); err != nil { + if err := internal.BatchDelete(ctx, r, l, key, + &networkingv1.Ingress{}, + &corev1.Service{}, + &v1.Deployment{}, + &corev1.PersistentVolumeClaim{}, + &corev1.ServiceAccount{}, + &corev1.ConfigMap{}, + ); err != nil { return err } diff --git a/internal/controller/core/site_controller.go b/internal/controller/core/site_controller.go index 471ad6a..fa71981 100644 --- a/internal/controller/core/site_controller.go +++ b/internal/controller/core/site_controller.go @@ -24,9 +24,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" ) -// isProductEnabled returns true if the product is enabled (nil defaults to enabled). -func isProductEnabled(b *bool) bool { - return b == nil || *b +// checkBool dereferences a bool pointer, returning defaultVal if nil. +func checkBool(b *bool, defaultVal bool) bool { + if b == nil { + return defaultVal + } + return *b } // SiteReconciler reconciles a Site object @@ -161,26 +164,26 @@ func (r *SiteReconciler) reconcileResources(ctx context.Context, req ctrl.Reques // VOLUMES // Determine if Connect is enabled (used for volume provisioning and later for reconciliation) - connectEnabled := site.Spec.Connect.Enabled == nil || *site.Spec.Connect.Enabled - connectTeardown := site.Spec.Connect.Teardown != nil && *site.Spec.Connect.Teardown + connectEnabled := checkBool(site.Spec.Connect.Enabled, true) + connectTeardown := checkBool(site.Spec.Connect.Teardown, false) if connectTeardown && connectEnabled { l.Info("connect.teardown is set but connect.enabled is not false; teardown has no effect until enabled=false") } - workbenchEnabled := isProductEnabled(site.Spec.Workbench.Enabled) - workbenchTeardown := site.Spec.Workbench.Teardown != nil && *site.Spec.Workbench.Teardown + workbenchEnabled := checkBool(site.Spec.Workbench.Enabled, true) + workbenchTeardown := checkBool(site.Spec.Workbench.Teardown, false) if workbenchTeardown && workbenchEnabled { l.Info("workbench.teardown is set but workbench.enabled is not false; teardown has no effect until enabled=false") } - pmEnabled := isProductEnabled(site.Spec.PackageManager.Enabled) - pmTeardown := site.Spec.PackageManager.Teardown != nil && *site.Spec.PackageManager.Teardown + pmEnabled := checkBool(site.Spec.PackageManager.Enabled, true) + pmTeardown := checkBool(site.Spec.PackageManager.Teardown, false) if pmTeardown && pmEnabled { l.Info("packageManager.teardown is set but packageManager.enabled is not false; teardown has no effect until enabled=false") } - chronicleEnabled := isProductEnabled(site.Spec.Chronicle.Enabled) - chronicleTeardown := site.Spec.Chronicle.Teardown != nil && *site.Spec.Chronicle.Teardown + chronicleEnabled := checkBool(site.Spec.Chronicle.Enabled, true) + chronicleTeardown := checkBool(site.Spec.Chronicle.Teardown, false) if chronicleTeardown && chronicleEnabled { l.Info("chronicle.teardown is set but chronicle.enabled is not false; teardown has no effect until enabled=false") } diff --git a/internal/controller/core/site_controller_networkpolicies.go b/internal/controller/core/site_controller_networkpolicies.go index 9dedc15..723ca41 100644 --- a/internal/controller/core/site_controller_networkpolicies.go +++ b/internal/controller/core/site_controller_networkpolicies.go @@ -39,7 +39,7 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. } // Chronicle network policy - chronicleEnabled := isProductEnabled(site.Spec.Chronicle.Enabled) + chronicleEnabled := checkBool(site.Spec.Chronicle.Enabled, true) if chronicleEnabled { if err := r.reconcileChronicleNetworkPolicy(ctx, req.Namespace, l, site); err != nil { l.Error(err, "error ensuring chronicle network policy") @@ -53,7 +53,7 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. } // Connect network policies - connectEnabled := site.Spec.Connect.Enabled == nil || *site.Spec.Connect.Enabled + connectEnabled := checkBool(site.Spec.Connect.Enabled, true) if connectEnabled { if err := r.reconcileConnectNetworkPolicy(ctx, req.Namespace, l, site); err != nil { l.Error(err, "error ensuring connect network policy") @@ -82,7 +82,7 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. } // Package Manager network policy - pmEnabled := isProductEnabled(site.Spec.PackageManager.Enabled) + pmEnabled := checkBool(site.Spec.PackageManager.Enabled, true) if pmEnabled { if err := r.reconcilePackageManagerNetworkPolicy(ctx, req.Namespace, l, site); err != nil { l.Error(err, "error ensuring package manager network policy") @@ -96,7 +96,7 @@ func (r *SiteReconciler) reconcileNetworkPolicies(ctx context.Context, req ctrl. } // Workbench network policies - workbenchEnabled := isProductEnabled(site.Spec.Workbench.Enabled) + workbenchEnabled := checkBool(site.Spec.Workbench.Enabled, true) if workbenchEnabled { if err := r.reconcileWorkbenchNetworkPolicy(ctx, req.Namespace, l, site); err != nil { l.Error(err, "error ensuring workbench network policy") diff --git a/internal/controller/core/workbench.go b/internal/controller/core/workbench.go index 01f7f6a..d981e8c 100644 --- a/internal/controller/core/workbench.go +++ b/internal/controller/core/workbench.go @@ -1034,22 +1034,11 @@ func (r *WorkbenchReconciler) CleanupWorkbench(ctx context.Context, req ctrl.Req func (r *WorkbenchReconciler) deleteServingResources(ctx context.Context, req ctrl.Request, w *positcov1beta1.Workbench, l logr.Logger) error { key := client.ObjectKey{Name: w.ComponentName(), Namespace: req.Namespace} - // INGRESS - if err := internal.BasicDelete(ctx, r, l, key, &networkingv1.Ingress{}); err != nil { - return err - } - - // SERVICE - if err := internal.BasicDelete(ctx, r, l, key, &corev1.Service{}); err != nil { - return err - } - - // DEPLOYMENT - if err := internal.BasicDelete(ctx, r, l, key, &appsv1.Deployment{}); err != nil { - return err - } - - return nil + return internal.BatchDelete(ctx, r, l, key, + &networkingv1.Ingress{}, + &corev1.Service{}, + &appsv1.Deployment{}, + ) } // suspendDeployedService removes serving resources (Deployment, Service, Ingress) diff --git a/internal/kubernetes_crudding.go b/internal/kubernetes_crudding.go index 60092ed..4a0b534 100644 --- a/internal/kubernetes_crudding.go +++ b/internal/kubernetes_crudding.go @@ -77,6 +77,16 @@ func BasicDelete(ctx context.Context, r product.SomeReconciler, l logr.Logger, k return nil } +// BatchDelete calls BasicDelete for each object in objects using the same key. Returns on the first error. +func BatchDelete(ctx context.Context, r product.SomeReconciler, l logr.Logger, key client.ObjectKey, objects ...client.Object) error { + for _, obj := range objects { + if err := BasicDelete(ctx, r, l, key, obj); err != nil { + return err + } + } + return nil +} + // PvcCreateOrUpdate is careful only to patch valid fields _if they have changed_. Otherwise, leave things // alone! In particular, StorageClassName will throw a diff every time if we leave it as blank (the default), because // Kubernetes fills in the StorageClassName From 654b36d66c91f63c0eeaf361349752ff8372bb4d Mon Sep 17 00:00:00 2001 From: ian-flores Date: Thu, 26 Feb 2026 17:33:51 -0800 Subject: [PATCH 11/11] fix: add secrets-store CRD to envtest paths for SecretProviderClass tests --- api/localtest/local.go | 1 + ...re.csi.x-k8s.io_secretproviderclasses.yaml | 177 ++++++++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 hack/secrets-store/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml diff --git a/api/localtest/local.go b/api/localtest/local.go index ed2fd33..21d3f87 100644 --- a/api/localtest/local.go +++ b/api/localtest/local.go @@ -24,6 +24,7 @@ func (lte *LocalTestEnv) makeEnv() error { filepath.Join(RootDir, "config", "crd", "bases"), filepath.Join(RootDir, "hack", "keycloak", "crds"), filepath.Join(RootDir, "hack", "traefik", "crds"), + filepath.Join(RootDir, "hack", "secrets-store", "crds"), }, ErrorIfCRDPathMissing: true, } diff --git a/hack/secrets-store/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml b/hack/secrets-store/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml new file mode 100644 index 0000000..fcf63c6 --- /dev/null +++ b/hack/secrets-store/crds/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml @@ -0,0 +1,177 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.12.1 + name: secretproviderclasses.secrets-store.csi.x-k8s.io +spec: + group: secrets-store.csi.x-k8s.io + names: + kind: SecretProviderClass + listKind: SecretProviderClassList + plural: secretproviderclasses + singular: secretproviderclass + scope: Namespaced + versions: + - name: v1 + schema: + openAPIV3Schema: + description: SecretProviderClass is the Schema for the secretproviderclasses + API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: SecretProviderClassSpec defines the desired state of SecretProviderClass + properties: + parameters: + additionalProperties: + type: string + description: Configuration for specific provider + type: object + provider: + description: Configuration for provider name + type: string + secretObjects: + items: + description: SecretObject defines the desired state of synced K8s + secret objects + properties: + annotations: + additionalProperties: + type: string + description: annotations of k8s secret object + type: object + data: + items: + description: SecretObjectData defines the desired state of + synced K8s secret object data + properties: + key: + description: data field to populate + type: string + objectName: + description: name of the object to sync + type: string + type: object + type: array + labels: + additionalProperties: + type: string + description: labels of K8s secret object + type: object + secretName: + description: name of the K8s secret object + type: string + type: + description: type of K8s secret object + type: string + type: object + type: array + type: object + status: + description: SecretProviderClassStatus defines the observed state of SecretProviderClass + type: object + type: object + served: true + storage: true + - deprecated: true + deprecationWarning: secrets-store.csi.x-k8s.io/v1alpha1 is deprecated. Use secrets-store.csi.x-k8s.io/v1 + instead. + name: v1alpha1 + schema: + openAPIV3Schema: + description: SecretProviderClass is the Schema for the secretproviderclasses + API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: SecretProviderClassSpec defines the desired state of SecretProviderClass + properties: + parameters: + additionalProperties: + type: string + description: Configuration for specific provider + type: object + provider: + description: Configuration for provider name + type: string + secretObjects: + items: + description: SecretObject defines the desired state of synced K8s + secret objects + properties: + annotations: + additionalProperties: + type: string + description: annotations of k8s secret object + type: object + data: + items: + description: SecretObjectData defines the desired state of + synced K8s secret object data + properties: + key: + description: data field to populate + type: string + objectName: + description: name of the object to sync + type: string + type: object + type: array + labels: + additionalProperties: + type: string + description: labels of K8s secret object + type: object + secretName: + description: name of the K8s secret object + type: string + type: + description: type of K8s secret object + type: string + type: object + type: array + type: object + status: + description: SecretProviderClassStatus defines the observed state of SecretProviderClass + properties: + byPod: + items: + description: ByPodStatus defines the state of SecretProviderClass + as seen by an individual controller + properties: + id: + description: id of the pod that wrote the status + type: string + namespace: + description: namespace of the pod that wrote the status + type: string + type: object + type: array + type: object + type: object + served: true + storage: false