From 2606ae2aab03aeb8de5ffacddc6cf86c44ceb77a Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Tue, 4 Jun 2024 15:49:25 -0400 Subject: [PATCH 01/17] tmp --- api/v1beta2/cryostat_types.go | 68 ++++- api/v1beta2/zz_generated.deepcopy.go | 42 +++ .../resource_definitions.go | 256 +++++++++++++++++- internal/controllers/constants/constants.go | 4 +- internal/controllers/reconciler.go | 97 +++++++ internal/controllers/reconciler_test.go | 57 +++- internal/controllers/services.go | 79 ++++++ internal/test/resources.go | 16 +- 8 files changed, 583 insertions(+), 36 deletions(-) diff --git a/api/v1beta2/cryostat_types.go b/api/v1beta2/cryostat_types.go index 3d8bca8e..a2ca64ca 100644 --- a/api/v1beta2/cryostat_types.go +++ b/api/v1beta2/cryostat_types.go @@ -203,6 +203,23 @@ type StorageConfiguration struct { // +optional // +operator-sdk:csv:customresourcedefinitions:type=spec EmptyDir *EmptyDirConfig `json:"emptyDir,omitempty"` + // Options to configure the Security Contexts for the storage to be created by the operator. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} + SecurityOptions *StorageSecurityOptions `json:"securityOptions,omitempty"` +} + +// StorageSecurityOptions contains Security Context customizations for the +// storage to be created by the operator at both the pod and container level. +type StorageSecurityOptions struct { + // Security Context to apply to the storage pod. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec + PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"` + // Security Context to apply to the storage container. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec + StorageSecurityContext *corev1.SecurityContext `json:"reportsSecurityContext,omitempty"` } // ReportConfiguration is used to determine how many replicas of cryostat-reports @@ -306,6 +323,26 @@ type ReportsServiceConfig struct { ServiceConfig `json:",inline"` } +// DatabaseServiceConfig provides customization for the service handling +// traffic for the cryostat application's database. +type DatabaseServiceConfig struct { + // HTTP port number for the cryostat application's database. + // Defaults to 5432. + // +optional + HTTPPort *int32 `json:"httpPort,omitempty"` + ServiceConfig `json:",inline"` +} + +// DatabaseServiceConfig provides customization for the service handling +// traffic for the storage to be created by the operator. +type StorageServiceConfig struct { + // HTTP port number for the storage to be created by the operator. + // Defaults to 8333. + // +optional + HTTPPort *int32 `json:"httpPort,omitempty"` + ServiceConfig `json:",inline"` +} + // ServiceConfigList holds the service configuration for each // service created by the operator. type ServiceConfigList struct { @@ -315,6 +352,12 @@ type ServiceConfigList struct { // Specification for the service responsible for the cryostat-reports sidecars. // +optional ReportsConfig *ReportsServiceConfig `json:"reportsConfig,omitempty"` + // Specification for the service responsible for the cryostat application's database. + // +optional + DatabaseConfig *DatabaseServiceConfig `json:"databaseConfig,omitempty"` + // Specification for the service responsible for the storage to be created by the operator. + // +optional + StorageConfig *StorageServiceConfig `json:"storageConfig,omitempty"` } // NetworkConfiguration provides customization for how to expose a Cryostat @@ -559,14 +602,6 @@ type SecurityOptions struct { // +optional // +operator-sdk:csv:customresourcedefinitions:type=spec GrafanaSecurityContext *corev1.SecurityContext `json:"grafanaSecurityContext,omitempty"` - // Security Context to apply to the storage container. - // +optional - // +operator-sdk:csv:customresourcedefinitions:type=spec - StorageSecurityContext *corev1.SecurityContext `json:"storageSecurityContext,omitempty"` - // Security Context to apply to the database container. - // +optional - // +operator-sdk:csv:customresourcedefinitions:type=spec - DatabaseSecurityContext *corev1.SecurityContext `json:"databaseSecurityContext,omitempty"` } // ReportsSecurityOptions contains Security Context customizations for the @@ -616,4 +651,21 @@ type DatabaseOptions struct { // +optional // +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:io.kubernetes:Secret"} SecretName *string `json:"secretName,omitempty"` + // Options to configure the Security Contexts for the Cryostat application's database. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} + SecurityOptions *DatabaseSecurityOptions `json:"securityOptions,omitempty"` +} + +// DatabaseSecurityOptions contains Security Context customizations for the +// Cryostat application's database at both the pod and container level. +type DatabaseSecurityOptions struct { + // Security Context to apply to the Cryostat application's database pod. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec + PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"` + // Security Context to apply to the Cryostat application's database container. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec + DatabaseSecurityContext *corev1.SecurityContext `json:"reportsSecurityContext,omitempty"` } diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index fe589e9c..baa47c67 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -570,6 +570,48 @@ func (in *ReportsServiceConfig) DeepCopy() *ReportsServiceConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DatabaseServiceConfig) DeepCopyInto(out *DatabaseServiceConfig) { + *out = *in + if in.HTTPPort != nil { + in, out := &in.HTTPPort, &out.HTTPPort + *out = new(int32) + **out = **in + } + in.ServiceConfig.DeepCopyInto(&out.ServiceConfig) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DatabaseServiceConfig. +func (in *DatabaseServiceConfig) DeepCopy() *DatabaseServiceConfig { + if in == nil { + return nil + } + out := new(DatabaseServiceConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *StorageServiceConfig) DeepCopyInto(out *StorageServiceConfig) { + *out = *in + if in.HTTPPort != nil { + in, out := &in.HTTPPort, &out.HTTPPort + *out = new(int32) + **out = **in + } + in.ServiceConfig.DeepCopyInto(&out.ServiceConfig) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StorageServiceConfig. +func (in *StorageServiceConfig) DeepCopy() *StorageServiceConfig { + if in == nil { + return nil + } + out := new(StorageServiceConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResourceConfigList) DeepCopyInto(out *ResourceConfigList) { *out = *in diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 1a4087b4..5bec0e1e 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -172,6 +172,168 @@ func NewDeploymentForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTa }, nil } +func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, + openshift bool) *appsv1.Deployment { + replicas := int32(1) + + defaultDeploymentLabels := map[string]string{ + "app": cr.Name, + "kind": "cryostat", + "component": "database", + "app.kubernetes.io/name": "cryostat-database", + } + defaultDeploymentAnnotations := map[string]string{ + "app.openshift.io/connects-to": cr.Name, + } + defaultPodLabels := map[string]string{ + "app": cr.Name, + "kind": "cryostat", + "component": "database", + } + userDefinedDeploymentLabels := make(map[string]string) + userDefinedDeploymentAnnotations := make(map[string]string) + userDefinedPodTemplateLabels := make(map[string]string) + userDefinedPodTemplateAnnotations := make(map[string]string) + if cr.Spec.OperandMetadata != nil { + if cr.Spec.OperandMetadata.DeploymentMetadata != nil { + for k, v := range cr.Spec.OperandMetadata.DeploymentMetadata.Labels { + userDefinedDeploymentLabels[k] = v + } + for k, v := range cr.Spec.OperandMetadata.DeploymentMetadata.Annotations { + userDefinedDeploymentAnnotations[k] = v + } + } + if cr.Spec.OperandMetadata.PodMetadata != nil { + for k, v := range cr.Spec.OperandMetadata.PodMetadata.Labels { + userDefinedPodTemplateLabels[k] = v + } + for k, v := range cr.Spec.OperandMetadata.PodMetadata.Annotations { + userDefinedPodTemplateAnnotations[k] = v + } + } + } + + // First set the user defined labels and annotation in the meta, so that the default ones can override them + deploymentMeta := metav1.ObjectMeta{ + Name: cr.Name + "-database", + Namespace: cr.InstallNamespace, + Labels: userDefinedDeploymentLabels, + Annotations: userDefinedDeploymentAnnotations, + } + common.MergeLabelsAndAnnotations(&deploymentMeta, defaultDeploymentLabels, defaultDeploymentAnnotations) + + podTemplateMeta := metav1.ObjectMeta{ + Name: cr.Name + "-database", + Namespace: cr.InstallNamespace, + Labels: userDefinedPodTemplateLabels, + Annotations: userDefinedPodTemplateAnnotations, + } + common.MergeLabelsAndAnnotations(&podTemplateMeta, defaultPodLabels, nil) + + return &appsv1.Deployment{ + ObjectMeta: deploymentMeta, + Spec: appsv1.DeploymentSpec{ + // Selector is immutable, avoid modifying if possible + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": cr.Name, + "kind": "cryostat", + "component": "database", + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: podTemplateMeta, + Spec: *NewPodForDatabase(cr, imageTags, openshift), + }, + Replicas: &replicas, + Strategy: appsv1.DeploymentStrategy{ + Type: appsv1.RecreateDeploymentStrategyType, + }, + }, + } +} + +func NewDeploymentForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, + openshift bool) *appsv1.Deployment { + replicas := int32(1) + + defaultDeploymentLabels := map[string]string{ + "app": cr.Name, + "kind": "cryostat", + "component": "storage", + "app.kubernetes.io/name": "cryostat-storage", + } + defaultDeploymentAnnotations := map[string]string{ + "app.openshift.io/connects-to": cr.Name, + } + defaultPodLabels := map[string]string{ + "app": cr.Name, + "kind": "cryostat", + "component": "storage", + } + userDefinedDeploymentLabels := make(map[string]string) + userDefinedDeploymentAnnotations := make(map[string]string) + userDefinedPodTemplateLabels := make(map[string]string) + userDefinedPodTemplateAnnotations := make(map[string]string) + if cr.Spec.OperandMetadata != nil { + if cr.Spec.OperandMetadata.DeploymentMetadata != nil { + for k, v := range cr.Spec.OperandMetadata.DeploymentMetadata.Labels { + userDefinedDeploymentLabels[k] = v + } + for k, v := range cr.Spec.OperandMetadata.DeploymentMetadata.Annotations { + userDefinedDeploymentAnnotations[k] = v + } + } + if cr.Spec.OperandMetadata.PodMetadata != nil { + for k, v := range cr.Spec.OperandMetadata.PodMetadata.Labels { + userDefinedPodTemplateLabels[k] = v + } + for k, v := range cr.Spec.OperandMetadata.PodMetadata.Annotations { + userDefinedPodTemplateAnnotations[k] = v + } + } + } + + // First set the user defined labels and annotation in the meta, so that the default ones can override them + deploymentMeta := metav1.ObjectMeta{ + Name: cr.Name + "-storage", + Namespace: cr.InstallNamespace, + Labels: userDefinedDeploymentLabels, + Annotations: userDefinedDeploymentAnnotations, + } + common.MergeLabelsAndAnnotations(&deploymentMeta, defaultDeploymentLabels, defaultDeploymentAnnotations) + + podTemplateMeta := metav1.ObjectMeta{ + Name: cr.Name + "-storage", + Namespace: cr.InstallNamespace, + Labels: userDefinedPodTemplateLabels, + Annotations: userDefinedPodTemplateAnnotations, + } + common.MergeLabelsAndAnnotations(&podTemplateMeta, defaultPodLabels, nil) + + return &appsv1.Deployment{ + ObjectMeta: deploymentMeta, + Spec: appsv1.DeploymentSpec{ + // Selector is immutable, avoid modifying if possible + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": cr.Name, + "kind": "cryostat", + "component": "storage", + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: podTemplateMeta, + Spec: *NewPodForStorage(cr, imageTags, tls, openshift), + }, + Replicas: &replicas, + Strategy: appsv1.DeploymentStrategy{ + Type: appsv1.RecreateDeploymentStrategyType, + }, + }, + } +} + func NewDeploymentForReports(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool) *appsv1.Deployment { replicas := int32(0) @@ -263,8 +425,6 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima NewCoreContainer(cr, specs, imageTags.CoreImageTag, tls, openshift), NewGrafanaContainer(cr, imageTags.GrafanaImageTag, tls), NewJfrDatasourceContainer(cr, imageTags.DatasourceImageTag), - NewStorageContainer(cr, imageTags.StorageImageTag, tls), - newDatabaseContainer(cr, imageTags.DatabaseImageTag, tls), *authProxy, } @@ -451,6 +611,86 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima }, nil } +func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, openshift bool) *corev1.PodSpec { + container := []corev1.Container{NewDatabaseContainer(cr, imageTags.DatabaseImageTag)} + + var podSc *corev1.PodSecurityContext + if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions.PodSecurityContext != nil { + podSc = cr.Spec.DatabaseOptions.SecurityOptions.PodSecurityContext + } else { + nonRoot := true + podSc = &corev1.PodSecurityContext{ + RunAsNonRoot: &nonRoot, + SeccompProfile: common.SeccompProfile(openshift), + } + } + + var nodeSelector map[string]string + var affinity *corev1.Affinity + var tolerations []corev1.Toleration + + if cr.Spec.DatabaseOptions != nil { + schedulingOptions := cr.Spec.SchedulingOptions + nodeSelector = schedulingOptions.NodeSelector + if schedulingOptions.Affinity != nil { + affinity = &corev1.Affinity{ + NodeAffinity: schedulingOptions.Affinity.NodeAffinity, + PodAffinity: schedulingOptions.Affinity.PodAffinity, + PodAntiAffinity: schedulingOptions.Affinity.PodAntiAffinity, + } + } + tolerations = schedulingOptions.Tolerations + } + + return &corev1.PodSpec{ + Containers: container, + NodeSelector: nodeSelector, + Affinity: affinity, + Tolerations: tolerations, + SecurityContext: podSc, + } +} + +func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool) *corev1.PodSpec { + container := []corev1.Container{NewStorageContainer(cr, imageTags.StorageImageTag)} + + var podSc *corev1.PodSecurityContext + if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SecurityOptions != nil && cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext != nil { + podSc = cr.Spec.StorageOptions.SecurityOptions.PodSecurityContext + } else { + nonRoot := true + podSc = &corev1.PodSecurityContext{ + RunAsNonRoot: &nonRoot, + SeccompProfile: common.SeccompProfile(openshift), + } + } + + var nodeSelector map[string]string + var affinity *corev1.Affinity + var tolerations []corev1.Toleration + + if cr.Spec.SchedulingOptions != nil { + nodeSelector = cr.Spec.SchedulingOptions.NodeSelector + + if cr.Spec.SchedulingOptions.Affinity != nil { + affinity = &corev1.Affinity{ + NodeAffinity: cr.Spec.SchedulingOptions.Affinity.NodeAffinity, + PodAffinity: cr.Spec.SchedulingOptions.Affinity.PodAffinity, + PodAntiAffinity: cr.Spec.SchedulingOptions.Affinity.PodAntiAffinity, + } + } + tolerations = cr.Spec.SchedulingOptions.Tolerations + } + + return &corev1.PodSpec{ + Containers: container, + NodeSelector: nodeSelector, + Affinity: affinity, + Tolerations: tolerations, + SecurityContext: podSc, + } +} + func NewReportContainerResource(cr *model.CryostatInstance) *corev1.ResourceRequirements { resources := &corev1.ResourceRequirements{} if cr.Spec.ReportOptions != nil { @@ -1278,7 +1518,7 @@ func NewStorageContainerResource(cr *model.CryostatInstance) *corev1.ResourceReq return resources } -func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSConfig) corev1.Container { +func NewStorageContainer(cr *model.CryostatInstance, imageTag string) corev1.Container { var containerSc *corev1.SecurityContext envs := []corev1.EnvVar{ { @@ -1322,8 +1562,8 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo }, }) - if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.StorageSecurityContext != nil { - containerSc = cr.Spec.SecurityOptions.StorageSecurityContext + if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SecurityOptions != nil && cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext != nil { + containerSc = cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext } else { privEscalation := false containerSc = &corev1.SecurityContext{ @@ -1376,10 +1616,10 @@ func NewDatabaseContainerResource(cr *model.CryostatInstance) *corev1.ResourceRe return resources } -func newDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSConfig) corev1.Container { +func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string) corev1.Container { var containerSc *corev1.SecurityContext - if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.DatabaseSecurityContext != nil { - containerSc = cr.Spec.SecurityOptions.DatabaseSecurityContext + if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions.DatabaseSecurityContext != nil { + containerSc = cr.Spec.DatabaseOptions.SecurityOptions.DatabaseSecurityContext } else { privEscalation := false containerSc = &corev1.SecurityContext{ diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index 86351dfc..3e9cfb90 100644 --- a/internal/controllers/constants/constants.go +++ b/internal/controllers/constants/constants.go @@ -25,8 +25,8 @@ const ( GrafanaContainerPort int32 = 3000 DatasourceContainerPort int32 = 8989 ReportsContainerPort int32 = 10000 - StoragePort int32 = 8333 - DatabasePort int32 = 5432 + StorageContainerPort int32 = 8333 + DatabaseContainerPort int32 = 5432 LoopbackAddress string = "127.0.0.1" OperatorNamePrefix string = "cryostat-operator-" OperatorDeploymentName string = "cryostat-operator-controller-manager" diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 8f4fd66a..f4229f9f 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -269,6 +269,16 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn return reportsResult, err } + storageResult, err := r.reconcileStorage(ctx, reqLogger, cr, tlsConfig, imageTags, serviceSpecs) + if err != nil { + return storageResult, err + } + + databaseResult, err := r.reconcileDatabase(ctx, reqLogger, cr, tlsConfig, imageTags, serviceSpecs) + if err != nil { + return dataBase, err + } + deployment, err := resources.NewDeploymentForCR(cr, serviceSpecs, imageTags, tlsConfig, *fsGroup, r.IsOpenShift) if err != nil { return reconcile.Result{}, err @@ -372,6 +382,93 @@ func (r *Reconciler) reconcileReports(ctx context.Context, reqLogger logr.Logger return reconcile.Result{}, nil } +func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, + tls *resources.TLSConfig, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs) (reconcile.Result, error) { + reqLogger.Info("Spec", "Database", cr.Spec.DatabaseOptions) + + desired := int32(1) + + err := r.reconcileReportsService(ctx, cr, tls, serviceSpecs) + if err != nil { + return reconcile.Result{}, err + } + deployment := resources.NewDeploymentForReports(cr, imageTags, tls, r.IsOpenShift) + if desired == 0 { + if err := r.Client.Delete(ctx, deployment); err != nil && !kerrors.IsNotFound(err) { + return reconcile.Result{}, err + } + + removeConditionIfPresent(cr, operatorv1beta1.ConditionTypeReportsDeploymentAvailable, + operatorv1beta1.ConditionTypeReportsDeploymentProgressing, + operatorv1beta1.ConditionTypeReportsDeploymentReplicaFailure) + err := r.Client.Status().Update(ctx, cr.Object) + if err != nil { + return reconcile.Result{}, err + } + return reconcile.Result{}, nil + } + + if desired > 0 { + err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) + if err != nil { + return reconcile.Result{}, err + } + + // Check deployment status and update conditions + err = r.updateConditionsFromDeployment(ctx, cr, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, + reportsDeploymentConditions) + if err != nil { + return reconcile.Result{}, err + } + } + return reconcile.Result{}, nil +} + +func (r *Reconciler) reconcileStorage(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, + tls *resources.TLSConfig, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs) (reconcile.Result, error) { + reqLogger.Info("Spec", "Reports", cr.Spec.ReportOptions) + + desired := int32(0) + if cr.Spec.ReportOptions != nil { + desired = cr.Spec.ReportOptions.Replicas + } + + err := r.reconcileReportsService(ctx, cr, tls, serviceSpecs) + if err != nil { + return reconcile.Result{}, err + } + deployment := resources.NewDeploymentForReports(cr, imageTags, tls, r.IsOpenShift) + if desired == 0 { + if err := r.Client.Delete(ctx, deployment); err != nil && !kerrors.IsNotFound(err) { + return reconcile.Result{}, err + } + + removeConditionIfPresent(cr, operatorv1beta1.ConditionTypeReportsDeploymentAvailable, + operatorv1beta1.ConditionTypeReportsDeploymentProgressing, + operatorv1beta1.ConditionTypeReportsDeploymentReplicaFailure) + err := r.Client.Status().Update(ctx, cr.Object) + if err != nil { + return reconcile.Result{}, err + } + return reconcile.Result{}, nil + } + + if desired > 0 { + err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) + if err != nil { + return reconcile.Result{}, err + } + + // Check deployment status and update conditions + err = r.updateConditionsFromDeployment(ctx, cr, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, + reportsDeploymentConditions) + if err != nil { + return reconcile.Result{}, err + } + } + return reconcile.Result{}, nil +} + func (r *Reconciler) getImageTags() *resources.ImageTags { return &resources.ImageTags{ OAuth2ProxyImageTag: r.getEnvOrDefault(oauth2ProxyImageTagEnv, DefaultOAuth2ProxyImageTag), diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index f03075e4..42071b23 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -944,11 +944,13 @@ func (c *controllerTest) commonTests() { }) It("should create deployment with the expected tags", func() { t.expectMainDeployment() + t.expectDatabaseDeployment() + t.expectStorageDeployment() t.checkReportsDeployment() }) It("should set ImagePullPolicy to Always", func() { containers := mainDeploy.Spec.Template.Spec.Containers - Expect(containers).To(HaveLen(6)) + Expect(containers).To(HaveLen(4)) for _, container := range containers { Expect(container.ImagePullPolicy).To(Equal(corev1.PullAlways)) } @@ -2660,16 +2662,8 @@ func (t *cryostatTestInput) checkMainPodTemplate(deployment *appsv1.Deployment, datasourceContainer := template.Spec.Containers[2] t.checkDatasourceContainer(&datasourceContainer, t.NewDatasourceContainerResource(cr), t.NewDatasourceSecurityContext(cr)) - // Check that Storage is configured properly - storageContainer := template.Spec.Containers[3] - t.checkStorageContainer(&storageContainer, t.NewStorageContainerResource(cr), t.NewStorageSecurityContext(cr)) - - // Check that Database is configured properly - databaseContainer := template.Spec.Containers[4] - t.checkDatabaseContainer(&databaseContainer, t.NewDatabaseContainerResource(cr), t.NewDatabaseSecurityContext(cr), dbSecretProvided) - // Check that Auth Proxy is configured properly - authProxyContainer := template.Spec.Containers[5] + authProxyContainer := template.Spec.Containers[3] t.checkAuthProxyContainer(&authProxyContainer, t.NewAuthProxyContainerResource(cr), t.NewAuthProxySecurityContext(cr), cr.Spec.AuthorizationOptions) // Check that the proper Service Account is set @@ -2702,6 +2696,49 @@ func (t *cryostatTestInput) expectMainPodTemplateHasExtraMetadata(deployment *ap })) } +func (t *cryostatTestInput) expectDatabaseDeployment() { + deployment := &appsv1.Deployment{} + err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-database", Namespace: t.Namespace}, deployment) + Expect(err).ToNot(HaveOccurred()) + + template := deployment.Spec.Template + Expect(template.Name).To(Equal(t.Name)) + Expect(template.Namespace).To(Equal(t.Namespace)) + Expect(template.Labels).To(Equal(map[string]string{ + "app": t.Name, + "kind": "cryostat", + "component": "database", + })) + Expect(template.Spec.Volumes).To(ConsistOf(t.NewDatabaseVolumes())) + Expect(template.Spec.SecurityContext).To(Equal(t.NewDatabasePodSecurityContext(cr))) + + // Check that Database is configured properly + databaseContainer := template.Spec.Containers[1] + t.checkDatabaseContainer(&databaseContainer, t.NewDatabaseContainerResource(cr), t.NewDatabaseSecurityContext(cr), dbSecretProvided) +} + +func (t *cryostatTestInput) expectStorageDeployment() { + deployment := &appsv1.Deployment{} + err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-storage", Namespace: t.Namespace}, deployment) + Expect(err).ToNot(HaveOccurred()) + + template := deployment.Spec.Template + Expect(template.Name).To(Equal(t.Name)) + Expect(template.Namespace).To(Equal(t.Namespace)) + Expect(template.Labels).To(Equal(map[string]string{ + "app": t.Name, + "kind": "cryostat", + "component": "database", + })) + Expect(template.Spec.Volumes).To(ConsistOf(t.NewStorageVolumes())) + Expect(template.Spec.SecurityContext).To(Equal(t.NewStoragePodSecurityContext(cr))) + + // Check that Storage is configured properly + storageContainer := template.Spec.Containers[1] + t.checkStorageContainer(&storageContainer, t.NewStorageContainerResource(cr), t.NewStorageSecurityContext(cr)) + +} + func (t *cryostatTestInput) checkReportsDeployment() { deployment := &appsv1.Deployment{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-reports", Namespace: t.Namespace}, deployment) diff --git a/internal/controllers/services.go b/internal/controllers/services.go index db099e1f..4e2776d8 100644 --- a/internal/controllers/services.go +++ b/internal/controllers/services.go @@ -111,6 +111,43 @@ func (r *Reconciler) reconcileReportsService(ctx context.Context, cr *model.Cryo return nil } +func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.CryostatInstance, + tls *resource_definitions.TLSConfig, specs *resource_definitions.ServiceSpecs) error { + config := configureDatabaseService(cr) + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name + "-database", + Namespace: cr.InstallNamespace, + }, + } + + err := r.createOrUpdateService(ctx, svc, cr.Object, &config.ServiceConfig, func() error { + svc.Spec.Selector = map[string]string{ + "app": cr.Name, + "component": "database", + } + svc.Spec.Ports = []corev1.ServicePort{ + { + Name: "http", + Port: *config.HTTPPort, + TargetPort: intstr.IntOrString{IntVal: constants.DatabaseContainerPort}, + }, + } + return nil + }) + if err != nil { + return err + } + + // Set reports URL for deployment to use + scheme := "http" + specs.ReportsURL = &url.URL{ + Scheme: scheme, + Host: svc.Name + ":" + strconv.Itoa(int(svc.Spec.Ports[0].Port)), // TODO use getHTTPPort? + } + return nil +} + func configureCoreService(cr *model.CryostatInstance) *operatorv1beta2.CoreServiceConfig { // Check CR for config var config *operatorv1beta2.CoreServiceConfig @@ -153,6 +190,48 @@ func configureReportsService(cr *model.CryostatInstance) *operatorv1beta2.Report return config } +func configureDatabaseService(cr *model.CryostatInstance) *operatorv1beta2.DatabaseServiceConfig { + // Check CR for config + var config *operatorv1beta2.DatabaseServiceConfig + if cr.Spec.ServiceOptions == nil || cr.Spec.ServiceOptions.DatabaseConfig == nil { + config = &operatorv1beta2.DatabaseServiceConfig{} + } else { + config = cr.Spec.ServiceOptions.DatabaseConfig.DeepCopy() + } + + // Apply common service defaults + configureService(&config.ServiceConfig, cr.Name, "database") + + // Apply default HTTP port if not provided + if config.HTTPPort == nil { + httpPort := constants.DatabaseContainerPort + config.HTTPPort = &httpPort + } + + return config +} + +func configureStorageService(cr *model.CryostatInstance) *operatorv1beta2.StorageServiceConfig { + // Check CR for config + var config *operatorv1beta2.StorageServiceConfig + if cr.Spec.ServiceOptions == nil || cr.Spec.ServiceOptions.StorageConfig == nil { + config = &operatorv1beta2.StorageServiceConfig{} + } else { + config = cr.Spec.ServiceOptions.StorageConfig.DeepCopy() + } + + // Apply common service defaults + configureService(&config.ServiceConfig, cr.Name, "storage") + + // Apply default HTTP port if not providednt + if config.HTTPPort == nil { + httpPort := constants.StorageContainerPort + config.HTTPPort = &httpPort + } + + return config +} + func configureService(config *operatorv1beta2.ServiceConfig, appLabel string, componentLabel string) { if config.ServiceType == nil { svcType := corev1.ServiceTypeClusterIP diff --git a/internal/test/resources.go b/internal/test/resources.go index 322f7311..81322b32 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -2294,23 +2294,23 @@ func (r *TestResources) NewDatasourceSecurityContext(cr *model.CryostatInstance) return r.commonDefaultSecurityContext() } -func (r *TestResources) NewStorageSecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { - if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.StorageSecurityContext != nil { - return cr.Spec.SecurityOptions.StorageSecurityContext +func (r *TestResources) NewAuthProxySecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { + if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.AuthProxySecurityContext != nil { + return cr.Spec.SecurityOptions.AuthProxySecurityContext } return r.commonDefaultSecurityContext() } func (r *TestResources) NewDatabaseSecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { - if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.DatabaseSecurityContext != nil { - return cr.Spec.SecurityOptions.DatabaseSecurityContext + if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions.DatabaseSecurityContext != nil { + return cr.Spec.DatabaseOptions.SecurityOptions.DatabaseSecurityContext } return r.commonDefaultSecurityContext() } -func (r *TestResources) NewAuthProxySecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { - if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.AuthProxySecurityContext != nil { - return cr.Spec.SecurityOptions.AuthProxySecurityContext +func (r *TestResources) NewStorageSecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { + if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SecurityOptions != nil && cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext != nil { + return cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext } return r.commonDefaultSecurityContext() } From 67c708ebff9f69c6544fee0ab4ea3d572c75159c Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Wed, 5 Jun 2024 14:46:45 -0400 Subject: [PATCH 02/17] tmp --- api/v1beta2/cryostat_types.go | 42 +----- config/scorecard/patches/custom.config.yaml | 12 +- .../resource_definitions.go | 57 +++---- internal/controllers/configmaps.go | 2 +- internal/controllers/reconciler.go | 94 ++++-------- internal/controllers/reconciler_test.go | 114 ++++++++++++-- internal/controllers/services.go | 43 +++++- internal/test/resources.go | 140 ++++++++++++++++++ 8 files changed, 355 insertions(+), 149 deletions(-) diff --git a/api/v1beta2/cryostat_types.go b/api/v1beta2/cryostat_types.go index a2ca64ca..a8625bb1 100644 --- a/api/v1beta2/cryostat_types.go +++ b/api/v1beta2/cryostat_types.go @@ -203,23 +203,6 @@ type StorageConfiguration struct { // +optional // +operator-sdk:csv:customresourcedefinitions:type=spec EmptyDir *EmptyDirConfig `json:"emptyDir,omitempty"` - // Options to configure the Security Contexts for the storage to be created by the operator. - // +optional - // +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} - SecurityOptions *StorageSecurityOptions `json:"securityOptions,omitempty"` -} - -// StorageSecurityOptions contains Security Context customizations for the -// storage to be created by the operator at both the pod and container level. -type StorageSecurityOptions struct { - // Security Context to apply to the storage pod. - // +optional - // +operator-sdk:csv:customresourcedefinitions:type=spec - PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"` - // Security Context to apply to the storage container. - // +optional - // +operator-sdk:csv:customresourcedefinitions:type=spec - StorageSecurityContext *corev1.SecurityContext `json:"reportsSecurityContext,omitempty"` } // ReportConfiguration is used to determine how many replicas of cryostat-reports @@ -602,6 +585,14 @@ type SecurityOptions struct { // +optional // +operator-sdk:csv:customresourcedefinitions:type=spec GrafanaSecurityContext *corev1.SecurityContext `json:"grafanaSecurityContext,omitempty"` + // Security Context to apply to the storage container. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec + StorageSecurityContext *corev1.SecurityContext `json:"storageSecurityContext,omitempty"` + // Security Context to apply to the database container. + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec + DatabaseSecurityContext *corev1.SecurityContext `json:"databaseSecurityContext,omitempty"` } // ReportsSecurityOptions contains Security Context customizations for the @@ -651,21 +642,4 @@ type DatabaseOptions struct { // +optional // +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:io.kubernetes:Secret"} SecretName *string `json:"secretName,omitempty"` - // Options to configure the Security Contexts for the Cryostat application's database. - // +optional - // +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} - SecurityOptions *DatabaseSecurityOptions `json:"securityOptions,omitempty"` -} - -// DatabaseSecurityOptions contains Security Context customizations for the -// Cryostat application's database at both the pod and container level. -type DatabaseSecurityOptions struct { - // Security Context to apply to the Cryostat application's database pod. - // +optional - // +operator-sdk:csv:customresourcedefinitions:type=spec - PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"` - // Security Context to apply to the Cryostat application's database container. - // +optional - // +operator-sdk:csv:customresourcedefinitions:type=spec - DatabaseSecurityContext *corev1.SecurityContext `json:"reportsSecurityContext,omitempty"` } diff --git a/config/scorecard/patches/custom.config.yaml b/config/scorecard/patches/custom.config.yaml index bada4b26..33bfcd54 100644 --- a/config/scorecard/patches/custom.config.yaml +++ b/config/scorecard/patches/custom.config.yaml @@ -8,7 +8,7 @@ entrypoint: - cryostat-scorecard-tests - operator-install - image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726" + image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253" labels: suite: cryostat test: operator-install @@ -18,7 +18,7 @@ entrypoint: - cryostat-scorecard-tests - cryostat-cr - image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726" + image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253" labels: suite: cryostat test: cryostat-cr @@ -28,7 +28,7 @@ entrypoint: - cryostat-scorecard-tests - cryostat-multi-namespace - image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726" + image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253" labels: suite: cryostat test: cryostat-multi-namespace @@ -38,7 +38,7 @@ entrypoint: - cryostat-scorecard-tests - cryostat-recording - image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726" + image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253" labels: suite: cryostat test: cryostat-recording @@ -48,7 +48,7 @@ entrypoint: - cryostat-scorecard-tests - cryostat-config-change - image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726" + image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253" labels: suite: cryostat test: cryostat-config-change @@ -58,7 +58,7 @@ entrypoint: - cryostat-scorecard-tests - cryostat-report - image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726" + image: "quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253" labels: suite: cryostat test: cryostat-report diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 5bec0e1e..85ccfb8e 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -172,8 +172,8 @@ func NewDeploymentForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTa }, nil } -func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, - openshift bool) *appsv1.Deployment { +func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, + openshift bool, fsGroup int64) *appsv1.Deployment { replicas := int32(1) defaultDeploymentLabels := map[string]string{ @@ -243,7 +243,7 @@ func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, }, Template: corev1.PodTemplateSpec{ ObjectMeta: podTemplateMeta, - Spec: *NewPodForDatabase(cr, imageTags, openshift), + Spec: *NewPodForDatabase(cr, imageTags, openshift, fsGroup), }, Replicas: &replicas, Strategy: appsv1.DeploymentStrategy{ @@ -253,8 +253,7 @@ func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, } } -func NewDeploymentForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, - openshift bool) *appsv1.Deployment { +func NewDeploymentForStorage(cr *model.CryostatInstance, imageTags *ImageTags, openshift bool, fsGroup int64) *appsv1.Deployment { replicas := int32(1) defaultDeploymentLabels := map[string]string{ @@ -324,7 +323,7 @@ func NewDeploymentForStorage(cr *model.CryostatInstance, imageTags *ImageTags, t }, Template: corev1.PodTemplateSpec{ ObjectMeta: podTemplateMeta, - Spec: *NewPodForStorage(cr, imageTags, tls, openshift), + Spec: *NewPodForStorage(cr, imageTags, openshift, fsGroup), }, Replicas: &replicas, Strategy: appsv1.DeploymentStrategy{ @@ -611,15 +610,17 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima }, nil } -func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, openshift bool) *corev1.PodSpec { +func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, openshift bool, fsGroup int64) *corev1.PodSpec { container := []corev1.Container{NewDatabaseContainer(cr, imageTags.DatabaseImageTag)} var podSc *corev1.PodSecurityContext - if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions.PodSecurityContext != nil { - podSc = cr.Spec.DatabaseOptions.SecurityOptions.PodSecurityContext + if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.PodSecurityContext != nil { + podSc = cr.Spec.SecurityOptions.PodSecurityContext } else { nonRoot := true podSc = &corev1.PodSecurityContext{ + // Ensure PV mounts are writable + FSGroup: &fsGroup, RunAsNonRoot: &nonRoot, SeccompProfile: common.SeccompProfile(openshift), } @@ -629,17 +630,17 @@ func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, openshi var affinity *corev1.Affinity var tolerations []corev1.Toleration - if cr.Spec.DatabaseOptions != nil { - schedulingOptions := cr.Spec.SchedulingOptions - nodeSelector = schedulingOptions.NodeSelector - if schedulingOptions.Affinity != nil { + if cr.Spec.SchedulingOptions != nil { + nodeSelector = cr.Spec.SchedulingOptions.NodeSelector + + if cr.Spec.SchedulingOptions.Affinity != nil { affinity = &corev1.Affinity{ - NodeAffinity: schedulingOptions.Affinity.NodeAffinity, - PodAffinity: schedulingOptions.Affinity.PodAffinity, - PodAntiAffinity: schedulingOptions.Affinity.PodAntiAffinity, + NodeAffinity: cr.Spec.SchedulingOptions.Affinity.NodeAffinity, + PodAffinity: cr.Spec.SchedulingOptions.Affinity.PodAffinity, + PodAntiAffinity: cr.Spec.SchedulingOptions.Affinity.PodAntiAffinity, } } - tolerations = schedulingOptions.Tolerations + tolerations = cr.Spec.SchedulingOptions.Tolerations } return &corev1.PodSpec{ @@ -651,15 +652,17 @@ func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, openshi } } -func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool) *corev1.PodSpec { +func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, openshift bool, fsGroup int64) *corev1.PodSpec { container := []corev1.Container{NewStorageContainer(cr, imageTags.StorageImageTag)} var podSc *corev1.PodSecurityContext - if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SecurityOptions != nil && cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext != nil { - podSc = cr.Spec.StorageOptions.SecurityOptions.PodSecurityContext + if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.PodSecurityContext != nil { + podSc = cr.Spec.SecurityOptions.PodSecurityContext } else { nonRoot := true podSc = &corev1.PodSecurityContext{ + // Ensure PV mounts are writable + FSGroup: &fsGroup, RunAsNonRoot: &nonRoot, SeccompProfile: common.SeccompProfile(openshift), } @@ -892,7 +895,7 @@ func NewOpenShiftAuthProxyContainer(cr *model.CryostatInstance, specs *ServiceSp "--pass-basic-auth=false", fmt.Sprintf("--upstream=http://localhost:%d/", constants.CryostatHTTPContainerPort), fmt.Sprintf("--upstream=http://localhost:%d/grafana/", constants.GrafanaContainerPort), - fmt.Sprintf("--upstream=http://localhost:%d/storage/", constants.StoragePort), + fmt.Sprintf("--upstream=http://localhost:%d/storage/", constants.StorageContainerPort), fmt.Sprintf("--openshift-service-account=%s", cr.Name), "--proxy-websockets=true", "--proxy-prefix=/oauth2", @@ -1562,8 +1565,8 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string) corev1.Con }, }) - if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SecurityOptions != nil && cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext != nil { - containerSc = cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext + if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.StorageSecurityContext != nil { + containerSc = cr.Spec.SecurityOptions.StorageSecurityContext } else { privEscalation := false containerSc = &corev1.SecurityContext{ @@ -1592,7 +1595,7 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string) corev1.Con Env: envs, Ports: []corev1.ContainerPort{ { - ContainerPort: constants.StoragePort, + ContainerPort: constants.StorageContainerPort, }, }, LivenessProbe: &corev1.Probe{ @@ -1618,8 +1621,8 @@ func NewDatabaseContainerResource(cr *model.CryostatInstance) *corev1.ResourceRe func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string) corev1.Container { var containerSc *corev1.SecurityContext - if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions.DatabaseSecurityContext != nil { - containerSc = cr.Spec.DatabaseOptions.SecurityOptions.DatabaseSecurityContext + if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.DatabaseSecurityContext != nil { + containerSc = cr.Spec.SecurityOptions.DatabaseSecurityContext } else { privEscalation := false containerSc = &corev1.SecurityContext{ @@ -1686,7 +1689,7 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string) corev1.Co Env: envs, Ports: []corev1.ContainerPort{ { - ContainerPort: constants.DatabasePort, + ContainerPort: constants.DatabaseContainerPort, }, }, ReadinessProbe: &corev1.Probe{ diff --git a/internal/controllers/configmaps.go b/internal/controllers/configmaps.go index adff1187..bd6f3d46 100644 --- a/internal/controllers/configmaps.go +++ b/internal/controllers/configmaps.go @@ -101,7 +101,7 @@ func (r *Reconciler) reconcileOAuth2ProxyConfig(ctx context.Context, cr *model.C Id: "storage", Path: "^/storage/(.*)$", RewriteTarget: "/$1", - Uri: fmt.Sprintf("http://localhost:%d", constants.StoragePort), + Uri: fmt.Sprintf("http://localhost:%d", constants.StorageContainerPort), PassHostHeader: false, ProxyWebSockets: false, }, diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index f4229f9f..fd6cefa5 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -269,14 +269,14 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn return reportsResult, err } - storageResult, err := r.reconcileStorage(ctx, reqLogger, cr, tlsConfig, imageTags, serviceSpecs) + databaseResult, err := r.reconcileDatabase(ctx, reqLogger, cr, imageTags, serviceSpecs, *fsGroup) if err != nil { - return storageResult, err + return databaseResult, err } - databaseResult, err := r.reconcileDatabase(ctx, reqLogger, cr, tlsConfig, imageTags, serviceSpecs) + storageResult, err := r.reconcileStorage(ctx, reqLogger, cr, imageTags, serviceSpecs, *fsGroup) if err != nil { - return dataBase, err + return storageResult, err } deployment, err := resources.NewDeploymentForCR(cr, serviceSpecs, imageTags, tlsConfig, *fsGroup, r.IsOpenShift) @@ -382,89 +382,49 @@ func (r *Reconciler) reconcileReports(ctx context.Context, reqLogger logr.Logger return reconcile.Result{}, nil } -func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, - tls *resources.TLSConfig, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs) (reconcile.Result, error) { +func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs, fsGroup int64) (reconcile.Result, error) { reqLogger.Info("Spec", "Database", cr.Spec.DatabaseOptions) - desired := int32(1) - - err := r.reconcileReportsService(ctx, cr, tls, serviceSpecs) + err := r.reconcileDatabaseService(ctx, cr, serviceSpecs) if err != nil { return reconcile.Result{}, err } - deployment := resources.NewDeploymentForReports(cr, imageTags, tls, r.IsOpenShift) - if desired == 0 { - if err := r.Client.Delete(ctx, deployment); err != nil && !kerrors.IsNotFound(err) { - return reconcile.Result{}, err - } + deployment := resources.NewDeploymentForDatabase(cr, imageTags, r.IsOpenShift) - removeConditionIfPresent(cr, operatorv1beta1.ConditionTypeReportsDeploymentAvailable, - operatorv1beta1.ConditionTypeReportsDeploymentProgressing, - operatorv1beta1.ConditionTypeReportsDeploymentReplicaFailure) - err := r.Client.Status().Update(ctx, cr.Object) - if err != nil { - return reconcile.Result{}, err - } - return reconcile.Result{}, nil + err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) + if err != nil { + return reconcile.Result{}, err } - if desired > 0 { - err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) - if err != nil { - return reconcile.Result{}, err - } - - // Check deployment status and update conditions - err = r.updateConditionsFromDeployment(ctx, cr, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, - reportsDeploymentConditions) - if err != nil { - return reconcile.Result{}, err - } + // Check deployment status and update conditions + err = r.updateConditionsFromDeployment(ctx, cr, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, + databaseDeploymentConditions) + if err != nil { + return reconcile.Result{}, err } return reconcile.Result{}, nil } func (r *Reconciler) reconcileStorage(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, - tls *resources.TLSConfig, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs) (reconcile.Result, error) { - reqLogger.Info("Spec", "Reports", cr.Spec.ReportOptions) + imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs, fsGroup int64) (reconcile.Result, error) { + reqLogger.Info("Spec", "Storage", cr.Spec.StorageOptions) - desired := int32(0) - if cr.Spec.ReportOptions != nil { - desired = cr.Spec.ReportOptions.Replicas - } - - err := r.reconcileReportsService(ctx, cr, tls, serviceSpecs) + err := r.reconcileStorageService(ctx, cr, serviceSpecs) if err != nil { return reconcile.Result{}, err } - deployment := resources.NewDeploymentForReports(cr, imageTags, tls, r.IsOpenShift) - if desired == 0 { - if err := r.Client.Delete(ctx, deployment); err != nil && !kerrors.IsNotFound(err) { - return reconcile.Result{}, err - } + deployment := resources.NewDeploymentForStorage(cr, imageTags, r.IsOpenShift, fsGroup) - removeConditionIfPresent(cr, operatorv1beta1.ConditionTypeReportsDeploymentAvailable, - operatorv1beta1.ConditionTypeReportsDeploymentProgressing, - operatorv1beta1.ConditionTypeReportsDeploymentReplicaFailure) - err := r.Client.Status().Update(ctx, cr.Object) - if err != nil { - return reconcile.Result{}, err - } - return reconcile.Result{}, nil + err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) + if err != nil { + return reconcile.Result{}, err } - if desired > 0 { - err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) - if err != nil { - return reconcile.Result{}, err - } - - // Check deployment status and update conditions - err = r.updateConditionsFromDeployment(ctx, cr, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, - reportsDeploymentConditions) - if err != nil { - return reconcile.Result{}, err - } + // Check deployment status and update conditions + err = r.updateConditionsFromDeployment(ctx, cr, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, + storageDeploymentConditions) + if err != nil { + return reconcile.Result{}, err } return reconcile.Result{}, nil } diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 42071b23..a65d99bd 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -149,6 +149,8 @@ func resourceChecks() []resourceCheck { {(*cryostatTestInput).expectStorageSecret, "object storage secret"}, {(*cryostatTestInput).expectCoreService, "core service"}, {(*cryostatTestInput).expectMainDeployment, "main deployment"}, + {(*cryostatTestInput).expectDatabaseDeployment, "database deployment"}, + {(*cryostatTestInput).expectStorageDeployment, "storage deployment"}, {(*cryostatTestInput).expectLockConfigMap, "lock config map"}, } } @@ -909,7 +911,7 @@ func (c *controllerTest) commonTests() { }) }) Context("with overriden image tags", func() { - var mainDeploy, reportsDeploy *appsv1.Deployment + var mainDeploy, databaseDeploy, storageDeploy, reportsDeploy *appsv1.Deployment BeforeEach(func() { t.ReportReplicas = 1 t.objs = append(t.objs, t.NewCryostatWithReportsSvc().Object) @@ -919,6 +921,12 @@ func (c *controllerTest) commonTests() { mainDeploy = &appsv1.Deployment{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name, Namespace: t.Namespace}, mainDeploy) Expect(err).ToNot(HaveOccurred()) + databaseDeploy = &appsv1.Deployment{} + err = t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-database", Namespace: t.Namespace}, databaseDeploy) + Expect(err).ToNot(HaveOccurred()) + storageDeploy = &appsv1.Deployment{} + err = t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-storage", Namespace: t.Namespace}, storageDeploy) + Expect(err).ToNot(HaveOccurred()) reportsDeploy = &appsv1.Deployment{} err = t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-reports", Namespace: t.Namespace}, reportsDeploy) Expect(err).ToNot(HaveOccurred()) @@ -954,6 +962,12 @@ func (c *controllerTest) commonTests() { for _, container := range containers { Expect(container.ImagePullPolicy).To(Equal(corev1.PullAlways)) } + databaseContainers := databaseDeploy.Spec.Template.Spec.Containers + Expect(databaseContainers).To(HaveLen(1)) + Expect(databaseContainers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) + storageContainers := storageDeploy.Spec.Template.Spec.Containers + Expect(storageContainers).To(HaveLen(1)) + Expect(storageContainers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) reportContainers := reportsDeploy.Spec.Template.Spec.Containers Expect(reportContainers).To(HaveLen(1)) Expect(reportContainers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) @@ -987,11 +1001,17 @@ func (c *controllerTest) commonTests() { }) It("should set ImagePullPolicy to IfNotPresent", func() { containers := mainDeploy.Spec.Template.Spec.Containers - Expect(containers).To(HaveLen(6)) + Expect(containers).To(HaveLen(4)) for _, container := range containers { fmt.Println(container.Image) Expect(container.ImagePullPolicy).To(Equal(corev1.PullIfNotPresent)) } + databaseContainers := databaseDeploy.Spec.Template.Spec.Containers + Expect(databaseContainers).To(HaveLen(1)) + Expect(databaseContainers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent)) + storageContainers := storageDeploy.Spec.Template.Spec.Containers + Expect(storageContainers).To(HaveLen(1)) + Expect(storageContainers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent)) reportContainers := reportsDeploy.Spec.Template.Spec.Containers Expect(reportContainers).To(HaveLen(1)) Expect(reportContainers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent)) @@ -1022,10 +1042,16 @@ func (c *controllerTest) commonTests() { }) It("should set ImagePullPolicy to IfNotPresent", func() { containers := mainDeploy.Spec.Template.Spec.Containers - Expect(containers).To(HaveLen(6)) + Expect(containers).To(HaveLen(4)) for _, container := range containers { Expect(container.ImagePullPolicy).To(Equal(corev1.PullIfNotPresent)) } + databaseContainers := databaseDeploy.Spec.Template.Spec.Containers + Expect(databaseContainers).To(HaveLen(1)) + Expect(databaseContainers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent)) + storageContainers := storageDeploy.Spec.Template.Spec.Containers + Expect(storageContainers).To(HaveLen(1)) + Expect(storageContainers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent)) reportContainers := reportsDeploy.Spec.Template.Spec.Containers Expect(reportContainers).To(HaveLen(1)) Expect(reportContainers[0].ImagePullPolicy).To(Equal(corev1.PullIfNotPresent)) @@ -1056,10 +1082,16 @@ func (c *controllerTest) commonTests() { }) It("should set ImagePullPolicy to Always", func() { containers := mainDeploy.Spec.Template.Spec.Containers - Expect(containers).To(HaveLen(6)) + Expect(containers).To(HaveLen(4)) for _, container := range containers { Expect(container.ImagePullPolicy).To(Equal(corev1.PullAlways), "Container %s", container.Image) } + databaseContainers := databaseDeploy.Spec.Template.Spec.Containers + Expect(databaseContainers).To(HaveLen(1)) + Expect(databaseContainers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) + storageContainers := storageDeploy.Spec.Template.Spec.Containers + Expect(storageContainers).To(HaveLen(1)) + Expect(storageContainers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) reportContainers := reportsDeploy.Spec.Template.Spec.Containers Expect(reportContainers).To(HaveLen(1)) Expect(reportContainers[0].ImagePullPolicy).To(Equal(corev1.PullAlways)) @@ -2701,20 +2733,51 @@ func (t *cryostatTestInput) expectDatabaseDeployment() { err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-database", Namespace: t.Namespace}, deployment) Expect(err).ToNot(HaveOccurred()) + cr := t.getCryostatInstance() + + Expect(deployment.Name).To(Equal(t.Name + "-database")) + Expect(deployment.Namespace).To(Equal(t.Namespace)) + Expect(deployment.Annotations).To(Equal(map[string]string{ + "app.openshift.io/connects-to": t.Name, + })) + Expect(deployment.Labels).To(Equal(map[string]string{ + "app": t.Name, + "kind": "cryostat", + "component": "database", + "app.kubernetes.io/name": "cryostat-database", + })) + Expect(deployment.Spec.Selector).To(Equal(t.NewDatabaseDeploymentSelector())) + + // compare Pod template template := deployment.Spec.Template - Expect(template.Name).To(Equal(t.Name)) + Expect(template.Name).To(Equal(t.Name + "-database")) Expect(template.Namespace).To(Equal(t.Namespace)) Expect(template.Labels).To(Equal(map[string]string{ "app": t.Name, "kind": "cryostat", "component": "database", })) - Expect(template.Spec.Volumes).To(ConsistOf(t.NewDatabaseVolumes())) Expect(template.Spec.SecurityContext).To(Equal(t.NewDatabasePodSecurityContext(cr))) // Check that Database is configured properly - databaseContainer := template.Spec.Containers[1] + dbSecretProvided := cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecretName != nil + databaseContainer := template.Spec.Containers[0] t.checkDatabaseContainer(&databaseContainer, t.NewDatabaseContainerResource(cr), t.NewDatabaseSecurityContext(cr), dbSecretProvided) + + // Check that the default Service Account is used + Expect(template.Spec.ServiceAccountName).To(BeEmpty()) + Expect(template.Spec.AutomountServiceAccountToken).To(BeNil()) + + if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SchedulingOptions != nil { + scheduling := cr.Spec.DatabaseOptions.SchedulingOptions + Expect(template.Spec.NodeSelector).To(Equal(scheduling.NodeSelector)) + if scheduling.Affinity != nil { + Expect(template.Spec.Affinity.PodAffinity).To(Equal(scheduling.Affinity.PodAffinity)) + Expect(template.Spec.Affinity.PodAntiAffinity).To(Equal(scheduling.Affinity.PodAntiAffinity)) + Expect(template.Spec.Affinity.NodeAffinity).To(Equal(scheduling.Affinity.NodeAffinity)) + } + Expect(template.Spec.Tolerations).To(Equal(scheduling.Tolerations)) + } } func (t *cryostatTestInput) expectStorageDeployment() { @@ -2722,21 +2785,50 @@ func (t *cryostatTestInput) expectStorageDeployment() { err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-storage", Namespace: t.Namespace}, deployment) Expect(err).ToNot(HaveOccurred()) + cr := t.getCryostatInstance() + + Expect(deployment.Name).To(Equal(t.Name + "-storage")) + Expect(deployment.Namespace).To(Equal(t.Namespace)) + Expect(deployment.Annotations).To(Equal(map[string]string{ + "app.openshift.io/connects-to": t.Name, + })) + Expect(deployment.Labels).To(Equal(map[string]string{ + "app": t.Name, + "kind": "cryostat", + "component": "storage", + "app.kubernetes.io/name": "cryostat-storage", + })) + Expect(deployment.Spec.Selector).To(Equal(t.NewStorageDeploymentSelector())) + + // compare Pod template template := deployment.Spec.Template - Expect(template.Name).To(Equal(t.Name)) + Expect(template.Name).To(Equal(t.Name + "-storage")) Expect(template.Namespace).To(Equal(t.Namespace)) Expect(template.Labels).To(Equal(map[string]string{ "app": t.Name, "kind": "cryostat", - "component": "database", + "component": "storage", })) - Expect(template.Spec.Volumes).To(ConsistOf(t.NewStorageVolumes())) Expect(template.Spec.SecurityContext).To(Equal(t.NewStoragePodSecurityContext(cr))) // Check that Storage is configured properly - storageContainer := template.Spec.Containers[1] + storageContainer := template.Spec.Containers[0] t.checkStorageContainer(&storageContainer, t.NewStorageContainerResource(cr), t.NewStorageSecurityContext(cr)) + // Check that the default Service Account is used + Expect(template.Spec.ServiceAccountName).To(BeEmpty()) + Expect(template.Spec.AutomountServiceAccountToken).To(BeNil()) + + if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SchedulingOptions != nil { + scheduling := cr.Spec.StorageOptions.SchedulingOptions + Expect(template.Spec.NodeSelector).To(Equal(scheduling.NodeSelector)) + if scheduling.Affinity != nil { + Expect(template.Spec.Affinity.PodAffinity).To(Equal(scheduling.Affinity.PodAffinity)) + Expect(template.Spec.Affinity.PodAntiAffinity).To(Equal(scheduling.Affinity.PodAntiAffinity)) + Expect(template.Spec.Affinity.NodeAffinity).To(Equal(scheduling.Affinity.NodeAffinity)) + } + Expect(template.Spec.Tolerations).To(Equal(scheduling.Tolerations)) + } } func (t *cryostatTestInput) checkReportsDeployment() { diff --git a/internal/controllers/services.go b/internal/controllers/services.go index 4e2776d8..4f166170 100644 --- a/internal/controllers/services.go +++ b/internal/controllers/services.go @@ -112,7 +112,7 @@ func (r *Reconciler) reconcileReportsService(ctx context.Context, cr *model.Cryo } func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.CryostatInstance, - tls *resource_definitions.TLSConfig, specs *resource_definitions.ServiceSpecs) error { + specs *resource_definitions.ServiceSpecs) error { config := configureDatabaseService(cr) svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -139,9 +139,46 @@ func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.Cry return err } - // Set reports URL for deployment to use + // Set database URL for deployment to use scheme := "http" - specs.ReportsURL = &url.URL{ + specs.DatabaseURL = &url.URL{ + Scheme: scheme, + Host: svc.Name + ":" + strconv.Itoa(int(svc.Spec.Ports[0].Port)), // TODO use getHTTPPort? + } + return nil +} + +func (r *Reconciler) reconcileStorageService(ctx context.Context, cr *model.CryostatInstance, + specs *resource_definitions.ServiceSpecs) error { + config := configureStorageService(cr) + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name + "-storage", + Namespace: cr.InstallNamespace, + }, + } + + err := r.createOrUpdateService(ctx, svc, cr.Object, &config.ServiceConfig, func() error { + svc.Spec.Selector = map[string]string{ + "app": cr.Name, + "component": "storage", + } + svc.Spec.Ports = []corev1.ServicePort{ + { + Name: "http", + Port: *config.HTTPPort, + TargetPort: intstr.IntOrString{IntVal: constants.StorageContainerPort}, + }, + } + return nil + }) + if err != nil { + return err + } + + // Set storage URL for deployment to use + scheme := "http" + specs.StorageURL = &url.URL{ Scheme: scheme, Host: svc.Name + ":" + strconv.Itoa(int(svc.Spec.Ports[0].Port)), // TODO use getHTTPPort? } diff --git a/internal/test/resources.go b/internal/test/resources.go index 81322b32..5febd561 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -105,10 +105,42 @@ func (r *TestResources) newCryostatSpec() operatorv1beta2.CryostatSpec { Replicas: r.ReportReplicas, } } + svcType := corev1.ServiceTypeNodePort + databaseHttpPort := int32(5432) + storageHttpPort := int32(8333) + serviceOptions := &operatorv1beta2.ServiceConfigList{ + DatabaseConfig: &operatorv1beta2.DatabaseServiceConfig{ + HTTPPort: &databaseHttpPort, + ServiceConfig: operatorv1beta2.ServiceConfig{ + ServiceType: &svcType, + Annotations: map[string]string{ + "my/custom": "annotation", + }, + Labels: map[string]string{ + "my": "label", + "app": "somethingelse", + }, + }, + }, + StorageConfig: &operatorv1beta2.StorageServiceConfig{ + HTTPPort: &storageHttpPort, + ServiceConfig: operatorv1beta2.ServiceConfig{ + ServiceType: &svcType, + Annotations: map[string]string{ + "my/custom": "annotation", + }, + Labels: map[string]string{ + "my": "label", + "app": "somethingelse", + }, + }, + }, + } return operatorv1beta2.CryostatSpec{ TargetNamespaces: r.TargetNamespaces, EnableCertManager: &certManager, ReportOptions: reportOptions, + ServiceOptions: serviceOptions, } } @@ -754,6 +786,80 @@ func (r *TestResources) NewCommandService() *corev1.Service { } } +func (r *TestResources) NewDatabaseService() *corev1.Service { + c := true + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.Name + "-database", + Namespace: r.Namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: operatorv1beta2.GroupVersion.String(), + Kind: "Cryostat", + Name: r.Name + "-database", + UID: "", + Controller: &c, + }, + }, + Labels: map[string]string{ + "app": r.Name, + "component": "database", + }, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Selector: map[string]string{ + "app": r.Name, + "component": "database", + }, + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: 5432, + TargetPort: intstr.FromInt(5432), + }, + }, + }, + } +} + +func (r *TestResources) NewStorageService() *corev1.Service { + c := true + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.Name + "-storage", + Namespace: r.Namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: operatorv1beta2.GroupVersion.String(), + Kind: "Cryostat", + Name: r.Name + "-storage", + UID: "", + Controller: &c, + }, + }, + Labels: map[string]string{ + "app": r.Name, + "component": "storage", + }, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Selector: map[string]string{ + "app": r.Name, + "component": "storage", + }, + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: 8333, + TargetPort: intstr.FromInt(8333), + }, + }, + }, + } +} + func (r *TestResources) NewReportsService() *corev1.Service { c := true return &corev1.Service{ @@ -1967,6 +2073,26 @@ func (r *TestResources) NewMainDeploymentSelector() *metav1.LabelSelector { } } +func (r *TestResources) NewDatabaseDeploymentSelector() *metav1.LabelSelector { + return &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": r.Name, + "kind": "cryostat", + "component": "database", + }, + } +} + +func (r *TestResources) NewStorageDeploymentSelector() *metav1.LabelSelector { + return &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": r.Name, + "kind": "cryostat", + "component": "storage", + }, + } +} + func (r *TestResources) NewReportsDeploymentSelector() *metav1.LabelSelector { return &metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -2273,6 +2399,20 @@ func (r *TestResources) NewReportPodSecurityContext(cr *model.CryostatInstance) return r.commonDefaultPodSecurityContext(nil) } +func (r *TestResources) NewDatabasePodSecurityContext(cr *model.CryostatInstance) *corev1.PodSecurityContext { + if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions.PodSecurityContext != nil { + return cr.Spec.DatabaseOptions.SecurityOptions.PodSecurityContext + } + return r.commonDefaultPodSecurityContext(nil) +} + +func (r *TestResources) NewStoragePodSecurityContext(cr *model.CryostatInstance) *corev1.PodSecurityContext { + if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SecurityOptions != nil && cr.Spec.StorageOptions.SecurityOptions.PodSecurityContext != nil { + return cr.Spec.StorageOptions.SecurityOptions.PodSecurityContext + } + return r.commonDefaultPodSecurityContext(nil) +} + func (r *TestResources) NewCoreSecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.CoreSecurityContext != nil { return cr.Spec.SecurityOptions.CoreSecurityContext From 509d8621e0028b4fffc479c5de35f889b2efb4a5 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Wed, 5 Jun 2024 16:32:13 -0400 Subject: [PATCH 03/17] tmp --- api/v1beta2/cryostat_types.go | 12 +++ api/v1beta2/zz_generated.deepcopy.go | 94 ++++++++++--------- .../operator.cryostat.io_cryostats.yaml | 52 ++++++++++ bundle/tests/scorecard/config.yaml | 12 +-- .../bases/operator.cryostat.io_cryostats.yaml | 52 ++++++++++ internal/controllers/reconciler.go | 12 ++- internal/controllers/reconciler_test.go | 12 +-- internal/test/resources.go | 54 +---------- 8 files changed, 195 insertions(+), 105 deletions(-) diff --git a/api/v1beta2/cryostat_types.go b/api/v1beta2/cryostat_types.go index a8625bb1..310225ed 100644 --- a/api/v1beta2/cryostat_types.go +++ b/api/v1beta2/cryostat_types.go @@ -179,6 +179,18 @@ const ( ConditionTypeMainDeploymentProgressing CryostatConditionType = "MainDeploymentProgressing" // If pods within the main Cryostat deployment failed to be created or destroyed. ConditionTypeMainDeploymentReplicaFailure CryostatConditionType = "MainDeploymentReplicaFailure" + // If enabled, whether the database deployment is available. + ConditionTypeDatabaseDeploymentAvailable CryostatConditionType = "DatabaseDeploymentAvailable" + // If enabled, whether the database deployment is progressing. + ConditionTypeDatabaseDeploymentProgressing CryostatConditionType = "DatabaseDeploymentProgressing" + // If enabled, whether pods in the database deployment failed to be created or destroyed. + ConditionTypeDatabaseDeploymentReplicaFailure CryostatConditionType = "DatabaseDeploymentReplicaFailure" + // If enabled, whether the storage deployment is available. + ConditionTypeStorageDeploymentAvailable CryostatConditionType = "StorageDeploymentAvailable" + // If enabled, whether the storage deployment is progressing. + ConditionTypeStorageDeploymentProgressing CryostatConditionType = "StorageDeploymentProgressing" + // If enabled, whether pods in the storage deployment failed to be created or destroyed. + ConditionTypeStorageDeploymentReplicaFailure CryostatConditionType = "StorageDeploymentReplicaFailure" // If enabled, whether the reports deployment is available. ConditionTypeReportsDeploymentAvailable CryostatConditionType = "ReportsDeploymentAvailable" // If enabled, whether the reports deployment is progressing. diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index baa47c67..afb3e77f 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -340,6 +340,27 @@ func (in *DatabaseOptions) DeepCopy() *DatabaseOptions { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DatabaseServiceConfig) DeepCopyInto(out *DatabaseServiceConfig) { + *out = *in + if in.HTTPPort != nil { + in, out := &in.HTTPPort, &out.HTTPPort + *out = new(int32) + **out = **in + } + in.ServiceConfig.DeepCopyInto(&out.ServiceConfig) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DatabaseServiceConfig. +func (in *DatabaseServiceConfig) DeepCopy() *DatabaseServiceConfig { + if in == nil { + return nil + } + out := new(DatabaseServiceConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EmptyDirConfig) DeepCopyInto(out *EmptyDirConfig) { *out = *in @@ -570,48 +591,6 @@ func (in *ReportsServiceConfig) DeepCopy() *ReportsServiceConfig { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *DatabaseServiceConfig) DeepCopyInto(out *DatabaseServiceConfig) { - *out = *in - if in.HTTPPort != nil { - in, out := &in.HTTPPort, &out.HTTPPort - *out = new(int32) - **out = **in - } - in.ServiceConfig.DeepCopyInto(&out.ServiceConfig) -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DatabaseServiceConfig. -func (in *DatabaseServiceConfig) DeepCopy() *DatabaseServiceConfig { - if in == nil { - return nil - } - out := new(DatabaseServiceConfig) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *StorageServiceConfig) DeepCopyInto(out *StorageServiceConfig) { - *out = *in - if in.HTTPPort != nil { - in, out := &in.HTTPPort, &out.HTTPPort - *out = new(int32) - **out = **in - } - in.ServiceConfig.DeepCopyInto(&out.ServiceConfig) -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StorageServiceConfig. -func (in *StorageServiceConfig) DeepCopy() *StorageServiceConfig { - if in == nil { - return nil - } - out := new(StorageServiceConfig) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResourceConfigList) DeepCopyInto(out *ResourceConfigList) { *out = *in @@ -818,6 +797,16 @@ func (in *ServiceConfigList) DeepCopyInto(out *ServiceConfigList) { *out = new(ReportsServiceConfig) (*in).DeepCopyInto(*out) } + if in.DatabaseConfig != nil { + in, out := &in.DatabaseConfig, &out.DatabaseConfig + *out = new(DatabaseServiceConfig) + (*in).DeepCopyInto(*out) + } + if in.StorageConfig != nil { + in, out := &in.StorageConfig, &out.StorageConfig + *out = new(StorageServiceConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceConfigList. @@ -855,6 +844,27 @@ func (in *StorageConfiguration) DeepCopy() *StorageConfiguration { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *StorageServiceConfig) DeepCopyInto(out *StorageServiceConfig) { + *out = *in + if in.HTTPPort != nil { + in, out := &in.HTTPPort, &out.HTTPPort + *out = new(int32) + **out = **in + } + in.ServiceConfig.DeepCopyInto(&out.ServiceConfig) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StorageServiceConfig. +func (in *StorageServiceConfig) DeepCopy() *StorageServiceConfig { + if in == nil { + return nil + } + out := new(StorageServiceConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TargetConnectionCacheOptions) DeepCopyInto(out *TargetConnectionCacheOptions) { *out = *in diff --git a/bundle/manifests/operator.cryostat.io_cryostats.yaml b/bundle/manifests/operator.cryostat.io_cryostats.yaml index bda0a2b6..c19c4044 100644 --- a/bundle/manifests/operator.cryostat.io_cryostats.yaml +++ b/bundle/manifests/operator.cryostat.io_cryostats.yaml @@ -8935,6 +8935,32 @@ spec: description: Type of service to create. Defaults to "ClusterIP". type: string type: object + databaseConfig: + description: Specification for the service responsible for the + cryostat application's database. + properties: + annotations: + additionalProperties: + type: string + description: Annotations to add to the service during its + creation. + type: object + httpPort: + description: HTTP port number for the cryostat application's + database. Defaults to 5432. + format: int32 + type: integer + labels: + additionalProperties: + type: string + description: Labels to add to the service during its creation. + The labels with keys "app" and "component" are reserved + for use by the operator. + type: object + serviceType: + description: Type of service to create. Defaults to "ClusterIP". + type: string + type: object reportsConfig: description: Specification for the service responsible for the cryostat-reports sidecars. @@ -8963,6 +8989,32 @@ spec: description: Type of service to create. Defaults to "ClusterIP". type: string type: object + storageConfig: + description: Specification for the service responsible for the + storage to be created by the operator. + properties: + annotations: + additionalProperties: + type: string + description: Annotations to add to the service during its + creation. + type: object + httpPort: + description: HTTP port number for the storage to be created + by the operator. Defaults to 8333. + format: int32 + type: integer + labels: + additionalProperties: + type: string + description: Labels to add to the service during its creation. + The labels with keys "app" and "component" are reserved + for use by the operator. + type: object + serviceType: + description: Type of service to create. Defaults to "ClusterIP". + type: string + type: object type: object storageOptions: description: Options to customize the storage provisioned for the diff --git a/bundle/tests/scorecard/config.yaml b/bundle/tests/scorecard/config.yaml index e88495e5..32ad65e2 100644 --- a/bundle/tests/scorecard/config.yaml +++ b/bundle/tests/scorecard/config.yaml @@ -70,7 +70,7 @@ stages: - entrypoint: - cryostat-scorecard-tests - operator-install - image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726 + image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253 labels: suite: cryostat test: operator-install @@ -80,7 +80,7 @@ stages: - entrypoint: - cryostat-scorecard-tests - cryostat-cr - image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726 + image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253 labels: suite: cryostat test: cryostat-cr @@ -90,7 +90,7 @@ stages: - entrypoint: - cryostat-scorecard-tests - cryostat-multi-namespace - image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726 + image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253 labels: suite: cryostat test: cryostat-multi-namespace @@ -100,7 +100,7 @@ stages: - entrypoint: - cryostat-scorecard-tests - cryostat-recording - image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726 + image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253 labels: suite: cryostat test: cryostat-recording @@ -110,7 +110,7 @@ stages: - entrypoint: - cryostat-scorecard-tests - cryostat-config-change - image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726 + image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253 labels: suite: cryostat test: cryostat-config-change @@ -120,7 +120,7 @@ stages: - entrypoint: - cryostat-scorecard-tests - cryostat-report - image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240511064726 + image: quay.io/cryostat/cryostat-operator-scorecard:3.0.0-20240605171253 labels: suite: cryostat test: cryostat-report diff --git a/config/crd/bases/operator.cryostat.io_cryostats.yaml b/config/crd/bases/operator.cryostat.io_cryostats.yaml index 16ebe539..00f7eda1 100644 --- a/config/crd/bases/operator.cryostat.io_cryostats.yaml +++ b/config/crd/bases/operator.cryostat.io_cryostats.yaml @@ -8922,6 +8922,32 @@ spec: description: Type of service to create. Defaults to "ClusterIP". type: string type: object + databaseConfig: + description: Specification for the service responsible for the + cryostat application's database. + properties: + annotations: + additionalProperties: + type: string + description: Annotations to add to the service during its + creation. + type: object + httpPort: + description: HTTP port number for the cryostat application's + database. Defaults to 5432. + format: int32 + type: integer + labels: + additionalProperties: + type: string + description: Labels to add to the service during its creation. + The labels with keys "app" and "component" are reserved + for use by the operator. + type: object + serviceType: + description: Type of service to create. Defaults to "ClusterIP". + type: string + type: object reportsConfig: description: Specification for the service responsible for the cryostat-reports sidecars. @@ -8950,6 +8976,32 @@ spec: description: Type of service to create. Defaults to "ClusterIP". type: string type: object + storageConfig: + description: Specification for the service responsible for the + storage to be created by the operator. + properties: + annotations: + additionalProperties: + type: string + description: Annotations to add to the service during its + creation. + type: object + httpPort: + description: HTTP port number for the storage to be created + by the operator. Defaults to 8333. + format: int32 + type: integer + labels: + additionalProperties: + type: string + description: Labels to add to the service during its creation. + The labels with keys "app" and "component" are reserved + for use by the operator. + type: object + serviceType: + description: Type of service to create. Defaults to "ClusterIP". + type: string + type: object type: object storageOptions: description: Options to customize the storage provisioned for the diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index fd6cefa5..b311b4d1 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -128,6 +128,16 @@ var mainDeploymentConditions = deploymentConditionTypeMap{ operatorv1beta2.ConditionTypeMainDeploymentProgressing: appsv1.DeploymentProgressing, operatorv1beta2.ConditionTypeMainDeploymentReplicaFailure: appsv1.DeploymentReplicaFailure, } +var databaseDeploymentConditions = deploymentConditionTypeMap{ + operatorv1beta2.ConditionTypeDatabaseDeploymentAvailable: appsv1.DeploymentAvailable, + operatorv1beta2.ConditionTypeDatabaseDeploymentProgressing: appsv1.DeploymentProgressing, + operatorv1beta2.ConditionTypeDatabaseDeploymentReplicaFailure: appsv1.DeploymentReplicaFailure, +} +var storageDeploymentConditions = deploymentConditionTypeMap{ + operatorv1beta2.ConditionTypeStorageDeploymentAvailable: appsv1.DeploymentAvailable, + operatorv1beta2.ConditionTypeStorageDeploymentProgressing: appsv1.DeploymentProgressing, + operatorv1beta2.ConditionTypeStorageDeploymentReplicaFailure: appsv1.DeploymentReplicaFailure, +} var reportsDeploymentConditions = deploymentConditionTypeMap{ operatorv1beta2.ConditionTypeReportsDeploymentAvailable: appsv1.DeploymentAvailable, operatorv1beta2.ConditionTypeReportsDeploymentProgressing: appsv1.DeploymentProgressing, @@ -389,7 +399,7 @@ func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logge if err != nil { return reconcile.Result{}, err } - deployment := resources.NewDeploymentForDatabase(cr, imageTags, r.IsOpenShift) + deployment := resources.NewDeploymentForDatabase(cr, imageTags, r.IsOpenShift, fsGroup) err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) if err != nil { diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index a65d99bd..fd8c4729 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -2757,7 +2757,7 @@ func (t *cryostatTestInput) expectDatabaseDeployment() { "kind": "cryostat", "component": "database", })) - Expect(template.Spec.SecurityContext).To(Equal(t.NewDatabasePodSecurityContext(cr))) + Expect(template.Spec.SecurityContext).To(Equal(t.NewPodSecurityContext(cr))) // Check that Database is configured properly dbSecretProvided := cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecretName != nil @@ -2768,8 +2768,8 @@ func (t *cryostatTestInput) expectDatabaseDeployment() { Expect(template.Spec.ServiceAccountName).To(BeEmpty()) Expect(template.Spec.AutomountServiceAccountToken).To(BeNil()) - if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SchedulingOptions != nil { - scheduling := cr.Spec.DatabaseOptions.SchedulingOptions + if cr.Spec.SchedulingOptions != nil { + scheduling := cr.Spec.SchedulingOptions Expect(template.Spec.NodeSelector).To(Equal(scheduling.NodeSelector)) if scheduling.Affinity != nil { Expect(template.Spec.Affinity.PodAffinity).To(Equal(scheduling.Affinity.PodAffinity)) @@ -2809,7 +2809,7 @@ func (t *cryostatTestInput) expectStorageDeployment() { "kind": "cryostat", "component": "storage", })) - Expect(template.Spec.SecurityContext).To(Equal(t.NewStoragePodSecurityContext(cr))) + Expect(template.Spec.SecurityContext).To(Equal(t.NewPodSecurityContext(cr))) // Check that Storage is configured properly storageContainer := template.Spec.Containers[0] @@ -2819,8 +2819,8 @@ func (t *cryostatTestInput) expectStorageDeployment() { Expect(template.Spec.ServiceAccountName).To(BeEmpty()) Expect(template.Spec.AutomountServiceAccountToken).To(BeNil()) - if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SchedulingOptions != nil { - scheduling := cr.Spec.StorageOptions.SchedulingOptions + if cr.Spec.SchedulingOptions != nil { + scheduling := cr.Spec.SchedulingOptions Expect(template.Spec.NodeSelector).To(Equal(scheduling.NodeSelector)) if scheduling.Affinity != nil { Expect(template.Spec.Affinity.PodAffinity).To(Equal(scheduling.Affinity.PodAffinity)) diff --git a/internal/test/resources.go b/internal/test/resources.go index 5febd561..25040eca 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -105,42 +105,10 @@ func (r *TestResources) newCryostatSpec() operatorv1beta2.CryostatSpec { Replicas: r.ReportReplicas, } } - svcType := corev1.ServiceTypeNodePort - databaseHttpPort := int32(5432) - storageHttpPort := int32(8333) - serviceOptions := &operatorv1beta2.ServiceConfigList{ - DatabaseConfig: &operatorv1beta2.DatabaseServiceConfig{ - HTTPPort: &databaseHttpPort, - ServiceConfig: operatorv1beta2.ServiceConfig{ - ServiceType: &svcType, - Annotations: map[string]string{ - "my/custom": "annotation", - }, - Labels: map[string]string{ - "my": "label", - "app": "somethingelse", - }, - }, - }, - StorageConfig: &operatorv1beta2.StorageServiceConfig{ - HTTPPort: &storageHttpPort, - ServiceConfig: operatorv1beta2.ServiceConfig{ - ServiceType: &svcType, - Annotations: map[string]string{ - "my/custom": "annotation", - }, - Labels: map[string]string{ - "my": "label", - "app": "somethingelse", - }, - }, - }, - } return operatorv1beta2.CryostatSpec{ TargetNamespaces: r.TargetNamespaces, EnableCertManager: &certManager, ReportOptions: reportOptions, - ServiceOptions: serviceOptions, } } @@ -2399,20 +2367,6 @@ func (r *TestResources) NewReportPodSecurityContext(cr *model.CryostatInstance) return r.commonDefaultPodSecurityContext(nil) } -func (r *TestResources) NewDatabasePodSecurityContext(cr *model.CryostatInstance) *corev1.PodSecurityContext { - if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions.PodSecurityContext != nil { - return cr.Spec.DatabaseOptions.SecurityOptions.PodSecurityContext - } - return r.commonDefaultPodSecurityContext(nil) -} - -func (r *TestResources) NewStoragePodSecurityContext(cr *model.CryostatInstance) *corev1.PodSecurityContext { - if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SecurityOptions != nil && cr.Spec.StorageOptions.SecurityOptions.PodSecurityContext != nil { - return cr.Spec.StorageOptions.SecurityOptions.PodSecurityContext - } - return r.commonDefaultPodSecurityContext(nil) -} - func (r *TestResources) NewCoreSecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.CoreSecurityContext != nil { return cr.Spec.SecurityOptions.CoreSecurityContext @@ -2442,15 +2396,15 @@ func (r *TestResources) NewAuthProxySecurityContext(cr *model.CryostatInstance) } func (r *TestResources) NewDatabaseSecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { - if cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions != nil && cr.Spec.DatabaseOptions.SecurityOptions.DatabaseSecurityContext != nil { - return cr.Spec.DatabaseOptions.SecurityOptions.DatabaseSecurityContext + if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.DatabaseSecurityContext != nil { + return cr.Spec.SecurityOptions.DatabaseSecurityContext } return r.commonDefaultSecurityContext() } func (r *TestResources) NewStorageSecurityContext(cr *model.CryostatInstance) *corev1.SecurityContext { - if cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.SecurityOptions != nil && cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext != nil { - return cr.Spec.StorageOptions.SecurityOptions.StorageSecurityContext + if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.StorageSecurityContext != nil { + return cr.Spec.SecurityOptions.StorageSecurityContext } return r.commonDefaultSecurityContext() } From 1815af879b74ffdec7fb6e4c3dca4dfce65ee31e Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Tue, 23 Jul 2024 10:30:14 -0400 Subject: [PATCH 04/17] separate db and storage into pods --- ...yostat-operator.clusterserviceversion.yaml | 4 +- .../operator.cryostat.io_cryostats.yaml | 16 +- .../bases/operator.cryostat.io_cryostats.yaml | 16 +- internal/controllers/certmanager.go | 18 +- .../resource_definitions/certificates.go | 48 +++++ .../resource_definitions.go | 135 +++++++++++-- internal/controllers/reconciler.go | 16 +- internal/controllers/reconciler_test.go | 108 ++++++---- internal/controllers/services.go | 15 +- internal/test/clients.go | 2 +- internal/test/resources.go | 184 +++++++++++++++++- 11 files changed, 470 insertions(+), 92 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 1d55dd8e..6c182dc1 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -30,7 +30,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-08-07T21:05:35Z" + createdAt: "2024-07-08T19:33:12Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { @@ -1018,7 +1018,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: quay.io/cryostat/cryostat-operator:4.0.0-dev + image: quay.io/miwan/cryostat-operator:4.0.0-dev imagePullPolicy: Always livenessProbe: httpGet: diff --git a/bundle/manifests/operator.cryostat.io_cryostats.yaml b/bundle/manifests/operator.cryostat.io_cryostats.yaml index c19c4044..c5412bc3 100644 --- a/bundle/manifests/operator.cryostat.io_cryostats.yaml +++ b/bundle/manifests/operator.cryostat.io_cryostats.yaml @@ -8946,14 +8946,16 @@ spec: creation. type: object httpPort: - description: HTTP port number for the cryostat application's - database. Defaults to 5432. + description: |- + HTTP port number for the cryostat application's database. + Defaults to 5432. format: int32 type: integer labels: additionalProperties: type: string - description: Labels to add to the service during its creation. + description: |- + Labels to add to the service during its creation. The labels with keys "app" and "component" are reserved for use by the operator. type: object @@ -9000,14 +9002,16 @@ spec: creation. type: object httpPort: - description: HTTP port number for the storage to be created - by the operator. Defaults to 8333. + description: |- + HTTP port number for the storage to be created by the operator. + Defaults to 8333. format: int32 type: integer labels: additionalProperties: type: string - description: Labels to add to the service during its creation. + description: |- + Labels to add to the service during its creation. The labels with keys "app" and "component" are reserved for use by the operator. type: object diff --git a/config/crd/bases/operator.cryostat.io_cryostats.yaml b/config/crd/bases/operator.cryostat.io_cryostats.yaml index 00f7eda1..d8dd5384 100644 --- a/config/crd/bases/operator.cryostat.io_cryostats.yaml +++ b/config/crd/bases/operator.cryostat.io_cryostats.yaml @@ -8933,14 +8933,16 @@ spec: creation. type: object httpPort: - description: HTTP port number for the cryostat application's - database. Defaults to 5432. + description: |- + HTTP port number for the cryostat application's database. + Defaults to 5432. format: int32 type: integer labels: additionalProperties: type: string - description: Labels to add to the service during its creation. + description: |- + Labels to add to the service during its creation. The labels with keys "app" and "component" are reserved for use by the operator. type: object @@ -8987,14 +8989,16 @@ spec: creation. type: object httpPort: - description: HTTP port number for the storage to be created - by the operator. Defaults to 8333. + description: |- + HTTP port number for the storage to be created by the operator. + Defaults to 8333. format: int32 type: integer labels: additionalProperties: type: string - description: Labels to add to the service during its creation. + description: |- + Labels to add to the service during its creation. The labels with keys "app" and "component" are reserved for use by the operator. type: object diff --git a/internal/controllers/certmanager.go b/internal/controllers/certmanager.go index 33c60623..90b32e45 100644 --- a/internal/controllers/certmanager.go +++ b/internal/controllers/certmanager.go @@ -89,12 +89,28 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) ( if err != nil { return nil, err } + + // Create a certificate for the Cryostat database signed by the Cryostat CA + databaseCert := resources.NewDatabaseCert(cr) + err = r.createOrUpdateCertificate(ctx, databaseCert, cr.Object) + if err != nil { + return nil, err + } + + // Create a certificate for Cryostat storage signed by the Cryostat CA + storageCert := resources.NewStorageCert(cr) + err = r.createOrUpdateCertificate(ctx, storageCert, cr.Object) + if err != nil { + return nil, err + } tlsConfig := &resources.TLSConfig{ CryostatSecret: cryostatCert.Spec.SecretName, ReportsSecret: reportsCert.Spec.SecretName, + DatabaseSecret: databaseCert.Spec.SecretName, + StorageSecret: storageCert.Spec.SecretName, KeystorePassSecret: cryostatCert.Spec.Keystores.PKCS12.PasswordSecretRef.Name, } - certificates := []*certv1.Certificate{caCert, cryostatCert, reportsCert} + certificates := []*certv1.Certificate{caCert, cryostatCert, reportsCert, databaseCert, storageCert} // Update owner references of TLS secrets created by cert-manager to ensure proper cleanup err = r.setCertSecretOwner(ctx, cr.Object, certificates...) diff --git a/internal/controllers/common/resource_definitions/certificates.go b/internal/controllers/common/resource_definitions/certificates.go index 9ba46b5f..5213eeea 100644 --- a/internal/controllers/common/resource_definitions/certificates.go +++ b/internal/controllers/common/resource_definitions/certificates.go @@ -131,3 +131,51 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate { }, } } + +func NewDatabaseCert(cr *model.CryostatInstance) *certv1.Certificate { + return &certv1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name + "-database", + Namespace: cr.InstallNamespace, + }, + Spec: certv1.CertificateSpec{ + CommonName: fmt.Sprintf("%s-database.%s.svc", cr.Name, cr.InstallNamespace), + DNSNames: []string{ + cr.Name + "-database", + fmt.Sprintf("%s-database.%s.svc", cr.Name, cr.InstallNamespace), + fmt.Sprintf("%s-database.%s.svc.cluster.local", cr.Name, cr.InstallNamespace), + }, + SecretName: cr.Name + "-database-tls", + IssuerRef: certMeta.ObjectReference{ + Name: cr.Name + "-ca", + }, + Usages: append(certv1.DefaultKeyUsages(), + certv1.UsageServerAuth, + ), + }, + } +} + +func NewStorageCert(cr *model.CryostatInstance) *certv1.Certificate { + return &certv1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name + "-storage", + Namespace: cr.InstallNamespace, + }, + Spec: certv1.CertificateSpec{ + CommonName: fmt.Sprintf("%s-storage.%s.svc", cr.Name, cr.InstallNamespace), + DNSNames: []string{ + cr.Name + "-storage", + fmt.Sprintf("%s-storage.%s.svc", cr.Name, cr.InstallNamespace), + fmt.Sprintf("%s-storage.%s.svc.cluster.local", cr.Name, cr.InstallNamespace), + }, + SecretName: cr.Name + "-storage-tls", + IssuerRef: certMeta.ObjectReference{ + Name: cr.Name + "-ca", + }, + Usages: append(certv1.DefaultKeyUsages(), + certv1.UsageServerAuth, + ), + }, + } +} diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 85ccfb8e..457b9d6a 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -61,6 +61,10 @@ type TLSConfig struct { CryostatSecret string // Name of the TLS secret for Reports Generator ReportsSecret string + // Name of the TLS secret for Database + DatabaseSecret string + // Name of the TLS secret for Storage + StorageSecret string // Name of the secret containing the password for the keystore in CryostatSecret KeystorePassSecret string // PEM-encoded X.509 certificate for the Cryostat CA @@ -172,7 +176,7 @@ func NewDeploymentForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTa }, nil } -func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, +func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool, fsGroup int64) *appsv1.Deployment { replicas := int32(1) @@ -243,7 +247,7 @@ func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, }, Template: corev1.PodTemplateSpec{ ObjectMeta: podTemplateMeta, - Spec: *NewPodForDatabase(cr, imageTags, openshift, fsGroup), + Spec: *NewPodForDatabase(cr, imageTags, tls, openshift, fsGroup), }, Replicas: &replicas, Strategy: appsv1.DeploymentStrategy{ @@ -253,7 +257,7 @@ func NewDeploymentForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, } } -func NewDeploymentForStorage(cr *model.CryostatInstance, imageTags *ImageTags, openshift bool, fsGroup int64) *appsv1.Deployment { +func NewDeploymentForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool, fsGroup int64) *appsv1.Deployment { replicas := int32(1) defaultDeploymentLabels := map[string]string{ @@ -323,7 +327,7 @@ func NewDeploymentForStorage(cr *model.CryostatInstance, imageTags *ImageTags, o }, Template: corev1.PodTemplateSpec{ ObjectMeta: podTemplateMeta, - Spec: *NewPodForStorage(cr, imageTags, openshift, fsGroup), + Spec: *NewPodForStorage(cr, imageTags, tls, openshift, fsGroup), }, Replicas: &replicas, Strategy: appsv1.DeploymentStrategy{ @@ -610,8 +614,21 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima }, nil } -func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, openshift bool, fsGroup int64) *corev1.PodSpec { - container := []corev1.Container{NewDatabaseContainer(cr, imageTags.DatabaseImageTag)} +func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool, fsGroup int64) *corev1.PodSpec { + container := []corev1.Container{NewDatabaseContainer(cr, imageTags.DatabaseImageTag, tls)} + + volumes := []corev1.Volume{} + if tls != nil { + secretVolume := corev1.Volume{ + Name: "database-tls-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: tls.DatabaseSecret, + }, + }, + } + volumes = append(volumes, secretVolume) + } var podSc *corev1.PodSecurityContext if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.PodSecurityContext != nil { @@ -649,11 +666,25 @@ func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, openshi Affinity: affinity, Tolerations: tolerations, SecurityContext: podSc, + Volumes: volumes, } } -func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, openshift bool, fsGroup int64) *corev1.PodSpec { - container := []corev1.Container{NewStorageContainer(cr, imageTags.StorageImageTag)} +func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool, fsGroup int64) *corev1.PodSpec { + container := []corev1.Container{NewStorageContainer(cr, imageTags.StorageImageTag, tls)} + + volumes := []corev1.Volume{} + if tls != nil { + secretVolume := corev1.Volume{ + Name: "storage-tls-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: tls.StorageSecret, + }, + }, + } + volumes = append(volumes, secretVolume) + } var podSc *corev1.PodSecurityContext if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.PodSecurityContext != nil { @@ -691,6 +722,7 @@ func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, openshif Affinity: affinity, Tolerations: tolerations, SecurityContext: podSc, + Volumes: volumes, } } @@ -1294,6 +1326,15 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag envs = append(envs, reportsEnvs...) } + envs = append(envs, corev1.EnvVar{ + Name: "CRYOSTAT_SERVICES_DATABASE_URL", + Value: specs.DatabaseURL.String(), + }) + envs = append(envs, corev1.EnvVar{ + Name: "CRYOSTAT_SERVICES_STORAGE_URL", + Value: specs.StorageURL.String(), + }) + // Define INSIGHTS_PROXY URL if Insights integration is enabled if specs.InsightsURL != nil { insightsEnvs := []corev1.EnvVar{ @@ -1521,7 +1562,7 @@ func NewStorageContainerResource(cr *model.CryostatInstance) *corev1.ResourceReq return resources } -func NewStorageContainer(cr *model.CryostatInstance, imageTag string) corev1.Container { +func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSConfig) corev1.Container { var containerSc *corev1.SecurityContext envs := []corev1.EnvVar{ { @@ -1565,6 +1606,44 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string) corev1.Con }, }) + livenessProbeScheme := corev1.URISchemeHTTP + if tls != nil { + tlsEnvs := []corev1.EnvVar{ + { + Name: "QUARKUS_HTTP_SSL_PORT", + Value: strconv.Itoa(int(constants.StorageContainerPort)), + }, + { + Name: "QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.StorageSecret, corev1.TLSPrivateKeyKey), + }, + { + Name: "QUARKUS_HTTP_SSL_CERTIFICATE_FILES", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.StorageSecret, corev1.TLSCertKey), + }, + { + Name: "QUARKUS_HTTP_INSECURE_REQUESTS", + Value: "disabled", + }, + } + + tlsSecretMount := corev1.VolumeMount{ + Name: "storage-tls-secret", + MountPath: "/var/run/secrets/operator.cryostat.io/" + tls.StorageSecret, + ReadOnly: true, + } + + envs = append(envs, tlsEnvs...) + mounts = append(mounts, tlsSecretMount) + + livenessProbeScheme = corev1.URISchemeHTTPS + } else { + envs = append(envs, corev1.EnvVar{ + Name: "QUARKUS_HTTP_PORT", + Value: strconv.Itoa(int(constants.StorageContainerPort)), + }) + } + if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.StorageSecurityContext != nil { containerSc = cr.Spec.SecurityOptions.StorageSecurityContext } else { @@ -1577,7 +1656,6 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string) corev1.Con } } - livenessProbeScheme := corev1.URISchemeHTTP probeHandler := corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Port: intstr.IntOrString{IntVal: 8333}, @@ -1619,7 +1697,7 @@ func NewDatabaseContainerResource(cr *model.CryostatInstance) *corev1.ResourceRe return resources } -func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string) corev1.Container { +func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSConfig) corev1.Container { var containerSc *corev1.SecurityContext if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.DatabaseSecurityContext != nil { containerSc = cr.Spec.SecurityOptions.DatabaseSecurityContext @@ -1680,6 +1758,41 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string) corev1.Co }, } + if tls != nil { + tlsEnvs := []corev1.EnvVar{ + { + Name: "QUARKUS_HTTP_SSL_PORT", + Value: strconv.Itoa(int(constants.DatabaseContainerPort)), + }, + { + Name: "QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.DatabaseSecret, corev1.TLSPrivateKeyKey), + }, + { + Name: "QUARKUS_HTTP_SSL_CERTIFICATE_FILES", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.DatabaseSecret, corev1.TLSCertKey), + }, + { + Name: "QUARKUS_HTTP_INSECURE_REQUESTS", + Value: "disabled", + }, + } + + tlsSecretMount := corev1.VolumeMount{ + Name: "database-tls-secret", + MountPath: "/var/run/secrets/operator.cryostat.io/" + tls.DatabaseSecret, + ReadOnly: true, + } + + envs = append(envs, tlsEnvs...) + mounts = append(mounts, tlsSecretMount) + } else { + envs = append(envs, corev1.EnvVar{ + Name: "QUARKUS_HTTP_PORT", + Value: strconv.Itoa(int(constants.DatabaseContainerPort)), + }) + } + return corev1.Container{ Name: cr.Name + "-db", Image: imageTag, diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index b311b4d1..0a38b469 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -279,12 +279,12 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn return reportsResult, err } - databaseResult, err := r.reconcileDatabase(ctx, reqLogger, cr, imageTags, serviceSpecs, *fsGroup) + databaseResult, err := r.reconcileDatabase(ctx, reqLogger, cr, tlsConfig, imageTags, serviceSpecs, *fsGroup) if err != nil { return databaseResult, err } - storageResult, err := r.reconcileStorage(ctx, reqLogger, cr, imageTags, serviceSpecs, *fsGroup) + storageResult, err := r.reconcileStorage(ctx, reqLogger, cr, tlsConfig, imageTags, serviceSpecs, *fsGroup) if err != nil { return storageResult, err } @@ -392,14 +392,14 @@ func (r *Reconciler) reconcileReports(ctx context.Context, reqLogger logr.Logger return reconcile.Result{}, nil } -func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs, fsGroup int64) (reconcile.Result, error) { +func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, tls *resources.TLSConfig, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs, fsGroup int64) (reconcile.Result, error) { reqLogger.Info("Spec", "Database", cr.Spec.DatabaseOptions) - err := r.reconcileDatabaseService(ctx, cr, serviceSpecs) + err := r.reconcileDatabaseService(ctx, cr, tls, serviceSpecs) if err != nil { return reconcile.Result{}, err } - deployment := resources.NewDeploymentForDatabase(cr, imageTags, r.IsOpenShift, fsGroup) + deployment := resources.NewDeploymentForDatabase(cr, imageTags, tls, r.IsOpenShift, fsGroup) err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) if err != nil { @@ -415,15 +415,15 @@ func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logge return reconcile.Result{}, nil } -func (r *Reconciler) reconcileStorage(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, +func (r *Reconciler) reconcileStorage(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, tls *resources.TLSConfig, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs, fsGroup int64) (reconcile.Result, error) { reqLogger.Info("Spec", "Storage", cr.Spec.StorageOptions) - err := r.reconcileStorageService(ctx, cr, serviceSpecs) + err := r.reconcileStorageService(ctx, cr, tls, serviceSpecs) if err != nil { return reconcile.Result{}, err } - deployment := resources.NewDeploymentForStorage(cr, imageTags, r.IsOpenShift, fsGroup) + deployment := resources.NewDeploymentForStorage(cr, imageTags, tls, r.IsOpenShift, fsGroup) err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) if err != nil { diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index fd8c4729..2e915d03 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -2276,7 +2276,7 @@ func (t *cryostatTestInput) expectWaitingForCertificate() { func (t *cryostatTestInput) expectCertificates() { // Check certificates - certs := []*certv1.Certificate{t.NewCryostatCert(), t.NewCACert(), t.NewReportsCert()} + certs := []*certv1.Certificate{t.NewCryostatCert(), t.NewCACert(), t.NewReportsCert(), t.NewDatabaseCert(), t.NewStorageCert()} for _, expected := range certs { actual := &certv1.Certificate{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: expected.Name, Namespace: expected.Namespace}, actual) @@ -2653,18 +2653,36 @@ func (t *cryostatTestInput) checkMainPodTemplate(deployment *appsv1.Deployment, // Check that the networking environment variables are set correctly coreContainer := template.Spec.Containers[0] - port := int32(10000) - if cr.Spec.ServiceOptions != nil && cr.Spec.ServiceOptions.ReportsConfig != nil && - cr.Spec.ServiceOptions.ReportsConfig.HTTPPort != nil { - port = *cr.Spec.ServiceOptions.ReportsConfig.HTTPPort + reportPort := int32(10000) + databasePort := int32(5432) + storagePort := int32(8333) + if cr.Spec.ServiceOptions != nil { + if cr.Spec.ServiceOptions.ReportsConfig != nil && cr.Spec.ServiceOptions.ReportsConfig.HTTPPort != nil { + reportPort = *cr.Spec.ServiceOptions.ReportsConfig.HTTPPort + } + if cr.Spec.ServiceOptions.DatabaseConfig != nil && cr.Spec.ServiceOptions.DatabaseConfig.HTTPPort != nil { + databasePort = *cr.Spec.ServiceOptions.DatabaseConfig.HTTPPort + } + if cr.Spec.ServiceOptions.StorageConfig != nil && cr.Spec.ServiceOptions.StorageConfig.HTTPPort != nil { + storagePort = *cr.Spec.ServiceOptions.StorageConfig.HTTPPort + } } var reportsUrl string + var databaseUrl string + var storageUrl string if t.ReportReplicas == 0 { reportsUrl = "" } else if t.TLS { - reportsUrl = fmt.Sprintf("https://%s-reports:%d", t.Name, port) + reportsUrl = fmt.Sprintf("https://%s-reports:%d", t.Name, reportPort) + } else { + reportsUrl = fmt.Sprintf("http://%s-reports:%d", t.Name, reportPort) + } + if t.TLS { + databaseUrl = fmt.Sprintf("https://%s-database:%d", t.Name, databasePort) + storageUrl = fmt.Sprintf("https://%s-storage:%d", t.Name, storagePort) } else { - reportsUrl = fmt.Sprintf("http://%s-reports:%d", t.Name, port) + databaseUrl = fmt.Sprintf("http://%s-database:%d", t.Name, databasePort) + storageUrl = fmt.Sprintf("http://%s-storage:%d", t.Name, storagePort) } ingress := !t.OpenShift && cr.Spec.NetworkOptions != nil && cr.Spec.NetworkOptions.CoreConfig != nil && cr.Spec.NetworkOptions.CoreConfig.IngressSpec != nil @@ -2678,7 +2696,7 @@ func (t *cryostatTestInput) checkMainPodTemplate(deployment *appsv1.Deployment, cr.Spec.TargetDiscoveryOptions.DisableBuiltInPortNumbers dbSecretProvided := cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecretName != nil - t.checkCoreContainer(&coreContainer, ingress, reportsUrl, + t.checkCoreContainer(&coreContainer, ingress, reportsUrl, databaseUrl, storageUrl, emptyDir, hasPortConfig, builtInDiscoveryDisabled, @@ -2757,6 +2775,7 @@ func (t *cryostatTestInput) expectDatabaseDeployment() { "kind": "cryostat", "component": "database", })) + Expect(template.Spec.Volumes).To(ConsistOf(t.NewDatabaseVolumes())) Expect(template.Spec.SecurityContext).To(Equal(t.NewPodSecurityContext(cr))) // Check that Database is configured properly @@ -2809,6 +2828,7 @@ func (t *cryostatTestInput) expectStorageDeployment() { "kind": "cryostat", "component": "storage", })) + Expect(template.Spec.Volumes).To(ConsistOf(t.NewStorageVolumes())) Expect(template.Spec.SecurityContext).To(Equal(t.NewPodSecurityContext(cr))) // Check that Storage is configured properly @@ -2932,6 +2952,8 @@ func (t *cryostatTestInput) checkDeploymentHasTemplates() { func (t *cryostatTestInput) checkCoreContainer(container *corev1.Container, ingress bool, reportsUrl string, + databaseUrl string, + storageUrl string, emptyDir bool, hasPortConfig bool, builtInDiscoveryDisabled bool, builtInPortConfigDisabled bool, dbSecretProvided bool, @@ -2944,7 +2966,7 @@ func (t *cryostatTestInput) checkCoreContainer(container *corev1.Container, ingr Expect(container.Image).To(Equal(*t.EnvCoreImageTag)) } Expect(container.Ports).To(ConsistOf(t.NewCorePorts())) - Expect(container.Env).To(ConsistOf(t.NewCoreEnvironmentVariables(reportsUrl, ingress, emptyDir, hasPortConfig, builtInDiscoveryDisabled, builtInPortConfigDisabled, dbSecretProvided))) + Expect(container.Env).To(ConsistOf(t.NewCoreEnvironmentVariables(reportsUrl, databaseUrl, storageUrl, ingress, emptyDir, hasPortConfig, builtInDiscoveryDisabled, builtInPortConfigDisabled, dbSecretProvided))) Expect(container.EnvFrom).To(ConsistOf(t.NewCoreEnvFromSource())) Expect(container.VolumeMounts).To(ConsistOf(t.NewCoreVolumeMounts())) Expect(container.LivenessProbe).To(Equal(t.NewCoreLivenessProbe())) @@ -2987,40 +3009,6 @@ func (t *cryostatTestInput) checkDatasourceContainer(container *corev1.Container test.ExpectResourceRequirements(&container.Resources, resources) } -func (t *cryostatTestInput) checkStorageContainer(container *corev1.Container, resources *corev1.ResourceRequirements, securityContext *corev1.SecurityContext) { - Expect(container.Name).To(Equal(t.Name + "-storage")) - if t.EnvStorageImageTag == nil { - Expect(container.Image).To(HavePrefix("quay.io/cryostat/cryostat-storage:")) - } else { - Expect(container.Image).To(Equal(*t.EnvStorageImageTag)) - } - Expect(container.Ports).To(ConsistOf(t.NewStoragePorts())) - Expect(container.Env).To(ConsistOf(t.NewStorageEnvironmentVariables())) - Expect(container.EnvFrom).To(BeEmpty()) - Expect(container.VolumeMounts).To(ConsistOf(t.NewStorageVolumeMounts())) - Expect(container.LivenessProbe).To(Equal(t.NewStorageLivenessProbe())) - Expect(container.SecurityContext).To(Equal(securityContext)) - - test.ExpectResourceRequirements(&container.Resources, resources) -} - -func (t *cryostatTestInput) checkDatabaseContainer(container *corev1.Container, resources *corev1.ResourceRequirements, securityContext *corev1.SecurityContext, dbSecretProvided bool) { - Expect(container.Name).To(Equal(t.Name + "-db")) - if t.EnvDatabaseImageTag == nil { - Expect(container.Image).To(HavePrefix("quay.io/cryostat/cryostat-db:")) - } else { - Expect(container.Image).To(Equal(*t.EnvDatabaseImageTag)) - } - Expect(container.Ports).To(ConsistOf(t.NewDatabasePorts())) - Expect(container.Env).To(ConsistOf(t.NewDatabaseEnvironmentVariables(dbSecretProvided))) - Expect(container.EnvFrom).To(BeEmpty()) - Expect(container.VolumeMounts).To(ConsistOf(t.NewDatabaseVolumeMounts())) - Expect(container.ReadinessProbe).To(Equal(t.NewDatabaseReadinessProbe())) - Expect(container.SecurityContext).To(Equal(securityContext)) - - test.ExpectResourceRequirements(&container.Resources, resources) -} - func (t *cryostatTestInput) checkAuthProxyContainer(container *corev1.Container, resources *corev1.ResourceRequirements, securityContext *corev1.SecurityContext, authOptions *operatorv1beta2.AuthorizationOptions) { Expect(container.Name).To(Equal(t.Name + "-auth-proxy")) @@ -3066,6 +3054,40 @@ func (t *cryostatTestInput) checkReportsContainer(container *corev1.Container, r test.ExpectResourceRequirements(&container.Resources, resources) } +func (t *cryostatTestInput) checkStorageContainer(container *corev1.Container, resources *corev1.ResourceRequirements, securityContext *corev1.SecurityContext) { + Expect(container.Name).To(Equal(t.Name + "-storage")) + if t.EnvStorageImageTag == nil { + Expect(container.Image).To(HavePrefix("quay.io/cryostat/cryostat-storage:")) + } else { + Expect(container.Image).To(Equal(*t.EnvStorageImageTag)) + } + Expect(container.Ports).To(ConsistOf(t.NewStoragePorts())) + Expect(container.Env).To(ConsistOf(t.NewStorageEnvironmentVariables())) + Expect(container.EnvFrom).To(BeEmpty()) + Expect(container.VolumeMounts).To(ConsistOf(t.NewStorageVolumeMounts())) + Expect(container.LivenessProbe).To(Equal(t.NewStorageLivenessProbe())) + Expect(container.SecurityContext).To(Equal(securityContext)) + + test.ExpectResourceRequirements(&container.Resources, resources) +} + +func (t *cryostatTestInput) checkDatabaseContainer(container *corev1.Container, resources *corev1.ResourceRequirements, securityContext *corev1.SecurityContext, dbSecretProvided bool) { + Expect(container.Name).To(Equal(t.Name + "-db")) + if t.EnvDatabaseImageTag == nil { + Expect(container.Image).To(HavePrefix("quay.io/cryostat/cryostat-db:")) + } else { + Expect(container.Image).To(Equal(*t.EnvDatabaseImageTag)) + } + Expect(container.Ports).To(ConsistOf(t.NewDatabasePorts())) + Expect(container.Env).To(ConsistOf(t.NewDatabaseEnvironmentVariables(dbSecretProvided))) + Expect(container.EnvFrom).To(BeEmpty()) + Expect(container.VolumeMounts).To(ConsistOf(t.NewDatabaseVolumeMounts())) + Expect(container.ReadinessProbe).To(Equal(t.NewDatabaseReadinessProbe())) + Expect(container.SecurityContext).To(Equal(securityContext)) + + test.ExpectResourceRequirements(&container.Resources, resources) +} + func (t *cryostatTestInput) checkCoreHasEnvironmentVariables(expectedEnvVars []corev1.EnvVar) { deployment := &appsv1.Deployment{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name, Namespace: t.Namespace}, deployment) diff --git a/internal/controllers/services.go b/internal/controllers/services.go index 4f166170..75e289a0 100644 --- a/internal/controllers/services.go +++ b/internal/controllers/services.go @@ -23,6 +23,7 @@ import ( operatorv1beta2 "github.com/cryostatio/cryostat-operator/api/v1beta2" common "github.com/cryostatio/cryostat-operator/internal/controllers/common" "github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions" + resources "github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions" "github.com/cryostatio/cryostat-operator/internal/controllers/constants" "github.com/cryostatio/cryostat-operator/internal/controllers/model" corev1 "k8s.io/api/core/v1" @@ -111,7 +112,7 @@ func (r *Reconciler) reconcileReportsService(ctx context.Context, cr *model.Cryo return nil } -func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.CryostatInstance, +func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.CryostatInstance, tls *resources.TLSConfig, specs *resource_definitions.ServiceSpecs) error { config := configureDatabaseService(cr) svc := &corev1.Service{ @@ -140,7 +141,10 @@ func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.Cry } // Set database URL for deployment to use - scheme := "http" + scheme := "https" + if tls == nil { + scheme = "http" + } specs.DatabaseURL = &url.URL{ Scheme: scheme, Host: svc.Name + ":" + strconv.Itoa(int(svc.Spec.Ports[0].Port)), // TODO use getHTTPPort? @@ -148,7 +152,7 @@ func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.Cry return nil } -func (r *Reconciler) reconcileStorageService(ctx context.Context, cr *model.CryostatInstance, +func (r *Reconciler) reconcileStorageService(ctx context.Context, cr *model.CryostatInstance, tls *resources.TLSConfig, specs *resource_definitions.ServiceSpecs) error { config := configureStorageService(cr) svc := &corev1.Service{ @@ -177,7 +181,10 @@ func (r *Reconciler) reconcileStorageService(ctx context.Context, cr *model.Cryo } // Set storage URL for deployment to use - scheme := "http" + scheme := "https" + if tls == nil { + scheme = "http" + } specs.StorageURL = &url.URL{ Scheme: scheme, Host: svc.Name + ":" + strconv.Itoa(int(svc.Spec.Ports[0].Port)), // TODO use getHTTPPort? diff --git a/internal/test/clients.go b/internal/test/clients.go index 4a46133e..4fa92c4c 100644 --- a/internal/test/clients.go +++ b/internal/test/clients.go @@ -70,7 +70,7 @@ func (c *testClient) makeCertificatesReady(ctx context.Context, obj runtime.Obje // If this object is one of the operator-managed certificates, mock the behaviour // of cert-manager processing those certificates cert, ok := obj.(*certv1.Certificate) - if ok && c.matchesName(cert, c.NewCryostatCert(), c.NewCACert(), c.NewReportsCert()) && + if ok && c.matchesName(cert, c.NewCryostatCert(), c.NewCACert(), c.NewReportsCert(), c.NewDatabaseCert(), c.NewStorageCert()) && len(cert.Status.Conditions) == 0 { // Create certificate secret c.createCertSecret(ctx, cert) diff --git a/internal/test/resources.go b/internal/test/resources.go index 25040eca..6b5badbc 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1066,6 +1066,58 @@ func (r *TestResources) NewReportsCert() *certv1.Certificate { } } +func (r *TestResources) NewDatabaseCert() *certv1.Certificate { + return &certv1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.Name + "-database", + Namespace: r.Namespace, + }, + Spec: certv1.CertificateSpec{ + CommonName: fmt.Sprintf(r.Name+"-database.%s.svc", r.Namespace), + DNSNames: []string{ + r.Name + "-database", + fmt.Sprintf(r.Name+"-database.%s.svc", r.Namespace), + fmt.Sprintf(r.Name+"-database.%s.svc.cluster.local", r.Namespace), + }, + SecretName: r.Name + "-database-tls", + IssuerRef: certMeta.ObjectReference{ + Name: r.Name + "-ca", + }, + Usages: []certv1.KeyUsage{ + certv1.UsageDigitalSignature, + certv1.UsageKeyEncipherment, + certv1.UsageServerAuth, + }, + }, + } +} + +func (r *TestResources) NewStorageCert() *certv1.Certificate { + return &certv1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.Name + "-storage", + Namespace: r.Namespace, + }, + Spec: certv1.CertificateSpec{ + CommonName: fmt.Sprintf(r.Name+"-storage.%s.svc", r.Namespace), + DNSNames: []string{ + r.Name + "-storage", + fmt.Sprintf(r.Name+"-storage.%s.svc", r.Namespace), + fmt.Sprintf(r.Name+"-storage.%s.svc.cluster.local", r.Namespace), + }, + SecretName: r.Name + "-storage-tls", + IssuerRef: certMeta.ObjectReference{ + Name: r.Name + "-ca", + }, + Usages: []certv1.KeyUsage{ + certv1.UsageDigitalSignature, + certv1.UsageKeyEncipherment, + certv1.UsageServerAuth, + }, + }, + } +} + func (r *TestResources) NewCACert() *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1273,7 +1325,7 @@ func (r *TestResources) NewAuthProxyPorts() []corev1.ContainerPort { } } -func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, ingress bool, +func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, databaseUrl string, storageUrl string, ingress bool, emptyDir bool, hasPortConfig bool, builtInDiscoveryDisabled bool, builtInPortConfigDisabled bool, dbSecretProvided bool) []corev1.EnvVar { envs := []corev1.EnvVar{ { @@ -1422,6 +1474,22 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, ingress b }) } + if databaseUrl != "" { + envs = append(envs, + corev1.EnvVar{ + Name: "CRYOSTAT_SERVICES_DATABASE_URL", + Value: databaseUrl, + }) + } + + if storageUrl != "" { + envs = append(envs, + corev1.EnvVar{ + Name: "CRYOSTAT_SERVICES_STORAGE_URL", + Value: storageUrl, + }) + } + if len(r.InsightsURL) > 0 { envs = append(envs, corev1.EnvVar{ @@ -1530,7 +1598,7 @@ func (r *TestResources) NewReportsEnvironmentVariables(resources *corev1.Resourc } func (r *TestResources) NewStorageEnvironmentVariables() []corev1.EnvVar { - return []corev1.EnvVar{ + envs := []corev1.EnvVar{ { Name: "CRYOSTAT_BUCKETS", Value: "archivedrecordings,archivedreports,eventtemplates,probes", @@ -1560,6 +1628,27 @@ func (r *TestResources) NewStorageEnvironmentVariables() []corev1.EnvVar { }, }, } + if r.TLS { + envs = append(envs, corev1.EnvVar{ + Name: "QUARKUS_HTTP_SSL_PORT", + Value: "8333", + }, corev1.EnvVar{ + Name: "QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls/tls.key", r.Name), + }, corev1.EnvVar{ + Name: "QUARKUS_HTTP_SSL_CERTIFICATE_FILES", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls/tls.crt", r.Name), + }, corev1.EnvVar{ + Name: "QUARKUS_HTTP_INSECURE_REQUESTS", + Value: "disabled", + }) + } else { + envs = append(envs, corev1.EnvVar{ + Name: "QUARKUS_HTTP_PORT", + Value: "8333", + }) + } + return envs } func (r *TestResources) NewDatabaseEnvironmentVariables(dbSecretProvided bool) []corev1.EnvVar { @@ -1568,7 +1657,7 @@ func (r *TestResources) NewDatabaseEnvironmentVariables(dbSecretProvided bool) [ if dbSecretProvided { secretName = providedDatabaseSecretName } - return []corev1.EnvVar{ + envs := []corev1.EnvVar{ { Name: "POSTGRESQL_USER", Value: "cryostat", @@ -1602,6 +1691,27 @@ func (r *TestResources) NewDatabaseEnvironmentVariables(dbSecretProvided bool) [ }, }, } + if r.TLS { + envs = append(envs, corev1.EnvVar{ + Name: "QUARKUS_HTTP_SSL_PORT", + Value: "5432", + }, corev1.EnvVar{ + Name: "QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls/tls.key", r.Name), + }, corev1.EnvVar{ + Name: "QUARKUS_HTTP_SSL_CERTIFICATE_FILES", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls/tls.crt", r.Name), + }, corev1.EnvVar{ + Name: "QUARKUS_HTTP_INSECURE_REQUESTS", + Value: "disabled", + }) + } else { + envs = append(envs, corev1.EnvVar{ + Name: "QUARKUS_HTTP_PORT", + Value: "5432", + }) + } + return envs } func (r *TestResources) NewAuthProxyEnvironmentVariables(authOptions *operatorv1beta2.AuthorizationOptions) []corev1.EnvVar { @@ -1833,23 +1943,41 @@ func (r *TestResources) NewCoreVolumeMounts() []corev1.VolumeMount { } func (r *TestResources) NewStorageVolumeMounts() []corev1.VolumeMount { - return []corev1.VolumeMount{ - { + mounts := []corev1.VolumeMount{} + mounts = append(mounts, + corev1.VolumeMount{ Name: r.Name, MountPath: "/data", SubPath: "seaweed", - }, + }) + if r.TLS { + mounts = append(mounts, + corev1.VolumeMount{ + Name: "storage-tls-secret", + MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls", r.Name), + ReadOnly: true, + }) } + return mounts } func (r *TestResources) NewDatabaseVolumeMounts() []corev1.VolumeMount { - return []corev1.VolumeMount{ - { + mounts := []corev1.VolumeMount{} + mounts = append(mounts, + corev1.VolumeMount{ Name: r.Name, MountPath: "/data", SubPath: "postgres", - }, + }) + if r.TLS { + mounts = append(mounts, + corev1.VolumeMount{ + Name: "database-tls-secret", + MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls", r.Name), + ReadOnly: true, + }) } + return mounts } func (r *TestResources) NewAuthProxyVolumeMounts(authOptions *operatorv1beta2.AuthorizationOptions) []corev1.VolumeMount { @@ -1973,12 +2101,16 @@ func (r *TestResources) NewDatasourceLivenessProbe() *corev1.Probe { } func (r *TestResources) NewStorageLivenessProbe() *corev1.Probe { + protocol := corev1.URISchemeHTTPS + if !r.TLS { + protocol = corev1.URISchemeHTTP + } return &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Port: intstr.IntOrString{IntVal: 8333}, Path: "/status", - Scheme: corev1.URISchemeHTTP, + Scheme: protocol, }, }, FailureThreshold: 2, @@ -2325,6 +2457,38 @@ func (r *TestResources) NewReportsVolumes() []corev1.Volume { } } +func (r *TestResources) NewDatabaseVolumes() []corev1.Volume { + if !r.TLS { + return nil + } + return []corev1.Volume{ + { + Name: "database-tls-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: r.Name + "-database-tls", + }, + }, + }, + } +} + +func (r *TestResources) NewStorageVolumes() []corev1.Volume { + if !r.TLS { + return nil + } + return []corev1.Volume{ + { + Name: "storage-tls-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: r.Name + "-storage-tls", + }, + }, + }, + } +} + func (r *TestResources) commonDefaultPodSecurityContext(fsGroup *int64) *corev1.PodSecurityContext { nonRoot := true var seccompProfile *corev1.SeccompProfile From fbe1eef6a29baa437fa6519401109334e3c35738 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Mon, 29 Jul 2024 16:28:27 -0400 Subject: [PATCH 05/17] tmp --- ...yostat-operator.clusterserviceversion.yaml | 7 +- config/manager/kustomization.yaml | 2 +- config/samples/operator_v1beta2_cryostat.yaml | 3 + .../resource_definitions.go | 58 ++++++---- internal/controllers/pvc.go | 34 +++++- internal/controllers/reconciler.go | 21 ++-- internal/controllers/reconciler_test.go | 108 ++++++++++++------ internal/controllers/services.go | 5 +- internal/test/resources.go | 72 +++++++----- 9 files changed, 212 insertions(+), 98 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 6c182dc1..8bf55efe 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -16,6 +16,11 @@ metadata: "reportOptions": { "replicas": 0 }, + "securityOptions": { + "authProxySecurityContext": { + "runAsUser": 1001 + } + }, "storageOptions": { "pvc": { "annotations": {}, @@ -30,7 +35,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-07-08T19:33:12Z" + createdAt: "2024-07-23T14:13:04Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 06415e71..78147e37 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -21,5 +21,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: quay.io/cryostat/cryostat-operator + newName: quay.io/miwan/cryostat-operator newTag: 4.0.0-dev diff --git a/config/samples/operator_v1beta2_cryostat.yaml b/config/samples/operator_v1beta2_cryostat.yaml index 8f014f31..d1823219 100644 --- a/config/samples/operator_v1beta2_cryostat.yaml +++ b/config/samples/operator_v1beta2_cryostat.yaml @@ -13,3 +13,6 @@ spec: spec: {} reportOptions: replicas: 0 + securityOptions: + authProxySecurityContext: + runAsUser: 1001 diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 457b9d6a..1b7f2c48 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -431,7 +431,7 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima *authProxy, } - volumes := newVolumeForCR(cr) + volumes := []corev1.Volume{} volSources := []corev1.VolumeProjection{} readOnlyMode := int32(0440) @@ -617,7 +617,7 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool, fsGroup int64) *corev1.PodSpec { container := []corev1.Container{NewDatabaseContainer(cr, imageTags.DatabaseImageTag, tls)} - volumes := []corev1.Volume{} + volumes := newVolumeForDatabse(cr) if tls != nil { secretVolume := corev1.Volume{ Name: "database-tls-secret", @@ -673,7 +673,7 @@ func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TL func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLSConfig, openshift bool, fsGroup int64) *corev1.PodSpec { container := []corev1.Container{NewStorageContainer(cr, imageTags.StorageImageTag, tls)} - volumes := []corev1.Volume{} + volumes := newVolumeForStorage(cr) if tls != nil { secretVolume := corev1.Volume{ Name: "storage-tls-secret", @@ -1635,7 +1635,6 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo envs = append(envs, tlsEnvs...) mounts = append(mounts, tlsSecretMount) - livenessProbeScheme = corev1.URISchemeHTTPS } else { envs = append(envs, corev1.EnvVar{ @@ -1874,19 +1873,6 @@ func NewJfrDatasourceContainer(cr *model.CryostatInstance, imageTag string) core } } -func getPort(url *url.URL) string { - // Return port if already defined in URL - port := url.Port() - if len(port) > 0 { - return port - } - // Otherwise use default HTTP(S) ports - if url.Scheme == "https" { - return "443" - } - return "80" -} - func getInternalDashboardURL() string { return fmt.Sprintf("http://localhost:%d", constants.GrafanaContainerPort) } @@ -1903,7 +1889,39 @@ func getPullPolicy(imageTag string) corev1.PullPolicy { return corev1.PullIfNotPresent } -func newVolumeForCR(cr *model.CryostatInstance) []corev1.Volume { +func newVolumeForDatabse(cr *model.CryostatInstance) []corev1.Volume { + var volumeSource corev1.VolumeSource + if useEmptyDir(cr) { + emptyDir := cr.Spec.StorageOptions.EmptyDir + + sizeLimit, err := resource.ParseQuantity(emptyDir.SizeLimit) + if err != nil { + sizeLimit = *resource.NewQuantity(0, resource.BinarySI) + } + + volumeSource = corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: emptyDir.Medium, + SizeLimit: &sizeLimit, + }, + } + } else { + volumeSource = corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: cr.Name + "-database", + }, + } + } + + return []corev1.Volume{ + { + Name: cr.Name + "-database", + VolumeSource: volumeSource, + }, + } +} + +func newVolumeForStorage(cr *model.CryostatInstance) []corev1.Volume { var volumeSource corev1.VolumeSource if useEmptyDir(cr) { emptyDir := cr.Spec.StorageOptions.EmptyDir @@ -1922,14 +1940,14 @@ func newVolumeForCR(cr *model.CryostatInstance) []corev1.Volume { } else { volumeSource = corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: cr.Name, + ClaimName: cr.Name + "-storage", }, } } return []corev1.Volume{ { - Name: cr.Name, + Name: cr.Name + "-storage", VolumeSource: volumeSource, }, } diff --git a/internal/controllers/pvc.go b/internal/controllers/pvc.go index 96c3cb1b..bc1d2007 100644 --- a/internal/controllers/pvc.go +++ b/internal/controllers/pvc.go @@ -31,7 +31,7 @@ import ( // Event type to inform users of invalid PVC specs const eventPersistentVolumeClaimInvalidType = "PersistentVolumeClaimInvalid" -func (r *Reconciler) reconcilePVC(ctx context.Context, cr *model.CryostatInstance) error { +func (r *Reconciler) reconcileDatabasePVC(ctx context.Context, cr *model.CryostatInstance) error { emptyDir := cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.EmptyDir != nil && cr.Spec.StorageOptions.EmptyDir.Enabled if emptyDir { // If user requested an emptyDir volume, then do nothing. @@ -41,7 +41,37 @@ func (r *Reconciler) reconcilePVC(ctx context.Context, cr *model.CryostatInstanc } pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - Name: cr.Name, + Name: cr.Name + "-database", + Namespace: cr.InstallNamespace, + }, + } + + // Look up PVC configuration, applying defaults where needed + config := configurePVC(cr) + + err := r.createOrUpdatePVC(ctx, pvc, cr.Object, config) + if err != nil { + // If the API server says the PVC is invalid, emit a warning event + // to inform the user. + if kerrors.IsInvalid(err) { + r.EventRecorder.Event(cr.Object, corev1.EventTypeWarning, eventPersistentVolumeClaimInvalidType, err.Error()) + } + return err + } + return nil +} + +func (r *Reconciler) reconcileStoragePVC(ctx context.Context, cr *model.CryostatInstance) error { + emptyDir := cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.EmptyDir != nil && cr.Spec.StorageOptions.EmptyDir.Enabled + if emptyDir { + // If user requested an emptyDir volume, then do nothing. + // Don't delete the PVC to prevent accidental data loss + // depending on the reclaim policy. + return nil + } + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name + "-storage", Namespace: cr.InstallNamespace, }, } diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 0a38b469..6a01608f 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -204,11 +204,6 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn return reconcile.Result{}, err } - err = r.reconcilePVC(ctx, cr) - if err != nil { - return reconcile.Result{}, err - } - err = r.reconcileSecrets(ctx, cr) if err != nil { return reconcile.Result{}, err @@ -395,12 +390,17 @@ func (r *Reconciler) reconcileReports(ctx context.Context, reqLogger logr.Logger func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logger, cr *model.CryostatInstance, tls *resources.TLSConfig, imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs, fsGroup int64) (reconcile.Result, error) { reqLogger.Info("Spec", "Database", cr.Spec.DatabaseOptions) - err := r.reconcileDatabaseService(ctx, cr, tls, serviceSpecs) + err := r.reconcileDatabasePVC(ctx, cr) + if err != nil { + return reconcile.Result{}, err + } + err = r.reconcileDatabaseService(ctx, cr, tls, serviceSpecs) if err != nil { return reconcile.Result{}, err } deployment := resources.NewDeploymentForDatabase(cr, imageTags, tls, r.IsOpenShift, fsGroup) + r.Log.Info(deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name) err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) if err != nil { return reconcile.Result{}, err @@ -419,7 +419,11 @@ func (r *Reconciler) reconcileStorage(ctx context.Context, reqLogger logr.Logger imageTags *resources.ImageTags, serviceSpecs *resources.ServiceSpecs, fsGroup int64) (reconcile.Result, error) { reqLogger.Info("Spec", "Storage", cr.Spec.StorageOptions) - err := r.reconcileStorageService(ctx, cr, tls, serviceSpecs) + err := r.reconcileStoragePVC(ctx, cr) + if err != nil { + return reconcile.Result{}, err + } + err = r.reconcileStorageService(ctx, cr, tls, serviceSpecs) if err != nil { return reconcile.Result{}, err } @@ -567,15 +571,18 @@ func (r *Reconciler) createOrUpdateDeployment(ctx context.Context, deploy *appsv // Update pod template spec to propagate any changes from Cryostat CR deploy.Spec.Template.Spec = deployCopy.Spec.Template.Spec + // Update pod template metadata common.MergeLabelsAndAnnotations(&deploy.Spec.Template.ObjectMeta, deployCopy.Spec.Template.Labels, deployCopy.Spec.Template.Annotations) return nil }) if err != nil { + r.Log.Info("sdfsfsdg err") if err == errSelectorModified { return r.recreateDeployment(ctx, deployCopy, owner) } + r.Log.Info(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name) return err } r.Log.Info(fmt.Sprintf("Deployment %s", op), "name", deploy.Name, "namespace", deploy.Namespace) diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 2e915d03..3375c9e6 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -37,7 +37,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/tools/record" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -143,8 +142,11 @@ func resourceChecks() []resourceCheck { {(*cryostatTestInput).expectRBAC, "RBAC"}, {(*cryostatTestInput).expectRoutes, "routes"}, {func(t *cryostatTestInput) { - t.expectPVC(t.NewDefaultPVC()) - }, "persistent volume claim"}, + t.expectPVC(t.NewDefaultPVC(t.Name+"-database"), t.Name+"-database") + }, "database persistent volume claim"}, + {func(t *cryostatTestInput) { + t.expectPVC(t.NewDefaultPVC(t.Name+"-storage"), t.Name+"-storage") + }, "storage persistent volume claim"}, {(*cryostatTestInput).expectDatabaseSecret, "database secret"}, {(*cryostatTestInput).expectStorageSecret, "object storage secret"}, {(*cryostatTestInput).expectCoreService, "core service"}, @@ -340,6 +342,8 @@ func (c *controllerTest) commonTests() { }) It("should delete and recreate the deployment", func() { t.expectMainDeployment() + t.expectDatabaseDeployment() + t.expectStorageDeployment() }) }) }) @@ -535,6 +539,8 @@ func (c *controllerTest) commonTests() { }) It("should configure deployment appropriately", func() { t.expectMainDeployment() + t.expectDatabaseDeployment() + t.expectStorageDeployment() t.checkReportsDeployment() t.checkService(t.Name+"-reports", t.NewReportsService()) }) @@ -554,6 +560,8 @@ func (c *controllerTest) commonTests() { }) It("should configure deployment appropriately", func() { t.expectMainDeployment() + t.expectDatabaseDeployment() + t.expectStorageDeployment() t.checkReportsDeployment() t.checkService(t.Name+"-reports", t.NewReportsService()) }) @@ -813,7 +821,8 @@ func (c *controllerTest) commonTests() { t.reconcileCryostatFully() }) It("should create the PVC with requested spec", func() { - t.expectPVC(t.NewCustomPVC()) + t.expectPVC(t.NewCustomPVC(t.Name+"-database"), t.Name+"-database") + t.expectPVC(t.NewCustomPVC(t.Name+"-storage"), t.Name+"-storage") }) }) Context("with custom PVC spec overriding some defaults", func() { @@ -824,7 +833,8 @@ func (c *controllerTest) commonTests() { t.reconcileCryostatFully() }) It("should create the PVC with requested spec", func() { - t.expectPVC(t.NewCustomPVCSomeDefault()) + t.expectPVC(t.NewCustomPVCSomeDefault(t.Name+"-database"), t.Name+"-database") + t.expectPVC(t.NewCustomPVCSomeDefault(t.Name+"-storage"), t.Name+"-storage") }) }) Context("with custom PVC config with no spec", func() { @@ -835,44 +845,61 @@ func (c *controllerTest) commonTests() { t.reconcileCryostatFully() }) It("should create the PVC with requested label", func() { - t.expectPVC(t.NewDefaultPVCWithLabel()) + t.expectPVC(t.NewDefaultPVCWithLabel(t.Name+"-database"), t.Name+"-database") + t.expectPVC(t.NewDefaultPVCWithLabel(t.Name+"-storage"), t.Name+"-storage") }) }) Context("with an existing PVC", func() { - var oldPVC *corev1.PersistentVolumeClaim + var oldDatabasePVC *corev1.PersistentVolumeClaim + var oldStoragePVC *corev1.PersistentVolumeClaim BeforeEach(func() { - oldPVC = t.NewDefaultPVC() - t.objs = append(t.objs, t.NewCryostatWithPVCSpec().Object, oldPVC) + oldDatabasePVC = t.NewDefaultPVC(t.Name + "-database") + oldStoragePVC = t.NewDefaultPVC(t.Name + "-storage") + t.objs = append(t.objs, t.NewCryostatWithPVCSpec().Object, oldDatabasePVC, oldStoragePVC) }) Context("that successfully updates", func() { BeforeEach(func() { // Add some labels and annotations to test merging - metav1.SetMetaDataLabel(&oldPVC.ObjectMeta, "my", "other-label") - metav1.SetMetaDataLabel(&oldPVC.ObjectMeta, "another", "label") - metav1.SetMetaDataAnnotation(&oldPVC.ObjectMeta, "my/custom", "other-annotation") - metav1.SetMetaDataAnnotation(&oldPVC.ObjectMeta, "another/custom", "annotation") + metav1.SetMetaDataLabel(&oldDatabasePVC.ObjectMeta, "my", "other-label") + metav1.SetMetaDataLabel(&oldDatabasePVC.ObjectMeta, "another", "label") + metav1.SetMetaDataAnnotation(&oldDatabasePVC.ObjectMeta, "my/custom", "other-annotation") + metav1.SetMetaDataAnnotation(&oldDatabasePVC.ObjectMeta, "another/custom", "annotation") + + metav1.SetMetaDataLabel(&oldStoragePVC.ObjectMeta, "my", "other-label") + metav1.SetMetaDataLabel(&oldStoragePVC.ObjectMeta, "another", "label") + metav1.SetMetaDataAnnotation(&oldStoragePVC.ObjectMeta, "my/custom", "other-annotation") + metav1.SetMetaDataAnnotation(&oldStoragePVC.ObjectMeta, "another/custom", "annotation") }) JustBeforeEach(func() { t.reconcileCryostatFully() }) It("should update metadata and resource requests", func() { - expected := t.NewDefaultPVC() - metav1.SetMetaDataLabel(&expected.ObjectMeta, "my", "label") - metav1.SetMetaDataLabel(&expected.ObjectMeta, "another", "label") - metav1.SetMetaDataLabel(&expected.ObjectMeta, "app", t.Name) - metav1.SetMetaDataAnnotation(&expected.ObjectMeta, "my/custom", "annotation") - metav1.SetMetaDataAnnotation(&expected.ObjectMeta, "another/custom", "annotation") - expected.Spec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse("10Gi") - t.expectPVC(expected) - }) - }) - Context("that fails to update", func() { + expectedDatabase := t.NewDefaultPVC(t.Name + "-database") + expectedStorage := t.NewDefaultPVC(t.Name + "-storage") + metav1.SetMetaDataLabel(&expectedDatabase.ObjectMeta, "my", "label") + metav1.SetMetaDataLabel(&expectedDatabase.ObjectMeta, "another", "label") + metav1.SetMetaDataLabel(&expectedDatabase.ObjectMeta, "app", t.Name) + metav1.SetMetaDataAnnotation(&expectedDatabase.ObjectMeta, "my/custom", "annotation") + metav1.SetMetaDataAnnotation(&expectedDatabase.ObjectMeta, "another/custom", "annotation") + expectedDatabase.Spec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse("10Gi") + t.expectPVC(expectedDatabase, t.Name+"-database") + + metav1.SetMetaDataLabel(&expectedStorage.ObjectMeta, "my", "label") + metav1.SetMetaDataLabel(&expectedStorage.ObjectMeta, "another", "label") + metav1.SetMetaDataLabel(&expectedStorage.ObjectMeta, "app", t.Name) + metav1.SetMetaDataAnnotation(&expectedStorage.ObjectMeta, "my/custom", "annotation") + metav1.SetMetaDataAnnotation(&expectedStorage.ObjectMeta, "another/custom", "annotation") + expectedStorage.Spec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse("10Gi") + t.expectPVC(expectedStorage, t.Name+"-storage") + }) + }) + /**Context("that fails to update", func() { JustBeforeEach(func() { // Replace client with one that fails to update the PVC - invalidErr := kerrors.NewInvalid(schema.ParseGroupKind("PersistentVolumeClaim"), oldPVC.Name, field.ErrorList{ + invalidErr := kerrors.NewInvalid(schema.ParseGroupKind("PersistentVolumeClaim"), oldDatabasePVC.Name, field.ErrorList{ field.Forbidden(field.NewPath("spec"), "test error"), }) - t.Client = test.NewClientWithUpdateError(t.Client, oldPVC, invalidErr) + t.Client = test.NewClientWithUpdateError(t.Client, oldDatabasePVC, invalidErr) t.controller.GetConfig().Client = t.Client // Expect an Invalid status error after reconciling @@ -886,7 +913,7 @@ func (c *controllerTest) commonTests() { Expect(recorder.Events).To(Receive(&eventMsg)) Expect(eventMsg).To(ContainSubstring("PersistentVolumeClaimInvalid")) }) - }) + })**/ }) Context("with custom EmptyDir config", func() { BeforeEach(func() { @@ -896,7 +923,8 @@ func (c *controllerTest) commonTests() { t.reconcileCryostatFully() }) It("should create the EmptyDir with default specs", func() { - t.expectEmptyDir(t.NewDefaultEmptyDir()) + t.expectDatabaseEmptyDir(t.NewDefaultEmptyDir()) + t.expectStorageEmptyDir(t.NewDefaultEmptyDir()) }) }) Context("with custom EmptyDir config with requested spec", func() { @@ -907,7 +935,8 @@ func (c *controllerTest) commonTests() { t.reconcileCryostatFully() }) It("should create the EmptyDir with requested specs", func() { - t.expectEmptyDir(t.NewEmptyDirWithSpec()) + t.expectDatabaseEmptyDir(t.NewEmptyDirWithSpec()) + t.expectStorageEmptyDir(t.NewEmptyDirWithSpec()) }) }) Context("with overriden image tags", func() { @@ -2401,9 +2430,9 @@ func (t *cryostatTestInput) expectLockConfigMap() { t.checkMetadata(cm, expected) } -func (t *cryostatTestInput) expectPVC(expectedPVC *corev1.PersistentVolumeClaim) { +func (t *cryostatTestInput) expectPVC(expectedPVC *corev1.PersistentVolumeClaim, name string) { pvc := &corev1.PersistentVolumeClaim{} - err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name, Namespace: t.Namespace}, pvc) + err := t.Client.Get(context.Background(), types.NamespacedName{Name: name, Namespace: t.Namespace}, pvc) Expect(err).ToNot(HaveOccurred()) // Compare to desired spec @@ -2421,9 +2450,22 @@ func (t *cryostatTestInput) expectPVC(expectedPVC *corev1.PersistentVolumeClaim) Expect(pvcStorage.Equal(expectedPVCStorage)).To(BeTrue()) } -func (t *cryostatTestInput) expectEmptyDir(expectedEmptyDir *corev1.EmptyDirVolumeSource) { +func (t *cryostatTestInput) expectDatabaseEmptyDir(expectedEmptyDir *corev1.EmptyDirVolumeSource) { deployment := &appsv1.Deployment{} - err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name, Namespace: t.Namespace}, deployment) + err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-database", Namespace: t.Namespace}, deployment) + Expect(err).ToNot(HaveOccurred()) + + volume := deployment.Spec.Template.Spec.Volumes[0] + emptyDir := volume.EmptyDir + + // Compare to desired spec + Expect(emptyDir.Medium).To(Equal(expectedEmptyDir.Medium)) + Expect(emptyDir.SizeLimit).To(Equal(expectedEmptyDir.SizeLimit)) +} + +func (t *cryostatTestInput) expectStorageEmptyDir(expectedEmptyDir *corev1.EmptyDirVolumeSource) { + deployment := &appsv1.Deployment{} + err := t.Client.Get(context.Background(), types.NamespacedName{Name: t.Name + "-storage", Namespace: t.Namespace}, deployment) Expect(err).ToNot(HaveOccurred()) volume := deployment.Spec.Template.Spec.Volumes[0] diff --git a/internal/controllers/services.go b/internal/controllers/services.go index 75e289a0..b2357beb 100644 --- a/internal/controllers/services.go +++ b/internal/controllers/services.go @@ -23,7 +23,6 @@ import ( operatorv1beta2 "github.com/cryostatio/cryostat-operator/api/v1beta2" common "github.com/cryostatio/cryostat-operator/internal/controllers/common" "github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions" - resources "github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions" "github.com/cryostatio/cryostat-operator/internal/controllers/constants" "github.com/cryostatio/cryostat-operator/internal/controllers/model" corev1 "k8s.io/api/core/v1" @@ -112,7 +111,7 @@ func (r *Reconciler) reconcileReportsService(ctx context.Context, cr *model.Cryo return nil } -func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.CryostatInstance, tls *resources.TLSConfig, +func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.CryostatInstance, tls *resource_definitions.TLSConfig, specs *resource_definitions.ServiceSpecs) error { config := configureDatabaseService(cr) svc := &corev1.Service{ @@ -152,7 +151,7 @@ func (r *Reconciler) reconcileDatabaseService(ctx context.Context, cr *model.Cry return nil } -func (r *Reconciler) reconcileStorageService(ctx context.Context, cr *model.CryostatInstance, tls *resources.TLSConfig, +func (r *Reconciler) reconcileStorageService(ctx context.Context, cr *model.CryostatInstance, tls *resource_definitions.TLSConfig, specs *resource_definitions.ServiceSpecs) error { config := configureStorageService(cr) svc := &corev1.Service{ diff --git a/internal/test/resources.go b/internal/test/resources.go index 6b5badbc..2d08aad6 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1182,10 +1182,10 @@ func (r *TestResources) OtherCAIssuer() *certv1.Issuer { } func (r *TestResources) newPVC(spec *corev1.PersistentVolumeClaimSpec, labels map[string]string, - annotations map[string]string) *corev1.PersistentVolumeClaim { + annotations map[string]string, name string) *corev1.PersistentVolumeClaim { return &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - Name: r.Name, + Name: name, Namespace: r.Namespace, Annotations: annotations, Labels: labels, @@ -1194,7 +1194,7 @@ func (r *TestResources) newPVC(spec *corev1.PersistentVolumeClaimSpec, labels ma } } -func (r *TestResources) NewDefaultPVC() *corev1.PersistentVolumeClaim { +func (r *TestResources) NewDefaultPVC(name string) *corev1.PersistentVolumeClaim { return r.newPVC(&corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.ResourceRequirements{ @@ -1204,10 +1204,10 @@ func (r *TestResources) NewDefaultPVC() *corev1.PersistentVolumeClaim { }, }, map[string]string{ "app": r.Name, - }, nil) + }, nil, name) } -func (r *TestResources) NewCustomPVC() *corev1.PersistentVolumeClaim { +func (r *TestResources) NewCustomPVC(name string) *corev1.PersistentVolumeClaim { storageClass := "cool-storage" return r.newPVC(&corev1.PersistentVolumeClaimSpec{ StorageClassName: &storageClass, @@ -1222,10 +1222,10 @@ func (r *TestResources) NewCustomPVC() *corev1.PersistentVolumeClaim { "app": r.Name, }, map[string]string{ "my/custom": "annotation", - }) + }, name) } -func (r *TestResources) NewCustomPVCSomeDefault() *corev1.PersistentVolumeClaim { +func (r *TestResources) NewCustomPVCSomeDefault(name string) *corev1.PersistentVolumeClaim { storageClass := "" return r.newPVC(&corev1.PersistentVolumeClaimSpec{ StorageClassName: &storageClass, @@ -1237,10 +1237,10 @@ func (r *TestResources) NewCustomPVCSomeDefault() *corev1.PersistentVolumeClaim }, }, map[string]string{ "app": r.Name, - }, nil) + }, nil, name) } -func (r *TestResources) NewDefaultPVCWithLabel() *corev1.PersistentVolumeClaim { +func (r *TestResources) NewDefaultPVCWithLabel(name string) *corev1.PersistentVolumeClaim { return r.newPVC(&corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.ResourceRequirements{ @@ -1251,7 +1251,7 @@ func (r *TestResources) NewDefaultPVCWithLabel() *corev1.PersistentVolumeClaim { }, map[string]string{ "app": r.Name, "my": "label", - }, nil) + }, nil, name) } func (r *TestResources) NewDefaultEmptyDir() *corev1.EmptyDirVolumeSource { @@ -2352,17 +2352,7 @@ func (r *TestResources) NewAuthPropertiesVolume() corev1.Volume { func (r *TestResources) newVolumes(certProjections []corev1.VolumeProjection) []corev1.Volume { readOnlymode := int32(0440) - volumes := []corev1.Volume{ - { - Name: r.Name, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: r.Name, - ReadOnly: false, - }, - }, - }, - } + volumes := []corev1.Volume{} projs := append([]corev1.VolumeProjection{}, certProjections...) if r.TLS { projs = append(projs, corev1.VolumeProjection{ @@ -2458,35 +2448,55 @@ func (r *TestResources) NewReportsVolumes() []corev1.Volume { } func (r *TestResources) NewDatabaseVolumes() []corev1.Volume { - if !r.TLS { - return nil - } - return []corev1.Volume{ + volumes := []corev1.Volume{ { + Name: r.Name + "-database", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: r.Name + "-database", + ReadOnly: false, + }, + }, + }, + } + + if r.TLS { + volumes = append(volumes, corev1.Volume{ Name: "database-tls-secret", VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ SecretName: r.Name + "-database-tls", }, }, - }, + }) } + return volumes } func (r *TestResources) NewStorageVolumes() []corev1.Volume { - if !r.TLS { - return nil - } - return []corev1.Volume{ + volumes := []corev1.Volume{ { + Name: r.Name + "-storage", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: r.Name + "-storage", + ReadOnly: false, + }, + }, + }, + } + + if r.TLS { + volumes = append(volumes, corev1.Volume{ Name: "storage-tls-secret", VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ SecretName: r.Name + "-storage-tls", }, }, - }, + }) } + return volumes } func (r *TestResources) commonDefaultPodSecurityContext(fsGroup *int64) *corev1.PodSecurityContext { From eb57ec8be03be0092281d14360117ccecaeef71f Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Mon, 29 Jul 2024 16:48:52 -0400 Subject: [PATCH 06/17] readd core PVC --- ...yostat-operator.clusterserviceversion.yaml | 2 +- .../resource_definitions.go | 38 +++++++++++- internal/controllers/pvc.go | 30 +++++++++ internal/controllers/reconciler.go | 8 ++- internal/controllers/reconciler_test.go | 62 +++++++------------ internal/test/resources.go | 58 +++++++++++++---- 6 files changed, 141 insertions(+), 57 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 8bf55efe..59fa2a9d 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -35,7 +35,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-07-23T14:13:04Z" + createdAt: "2024-07-29T20:27:49Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 1b7f2c48..6c62a43d 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -431,7 +431,7 @@ func NewPodForCR(cr *model.CryostatInstance, specs *ServiceSpecs, imageTags *Ima *authProxy, } - volumes := []corev1.Volume{} + volumes := newVolumeForCR(cr) volSources := []corev1.VolumeProjection{} readOnlyMode := int32(0440) @@ -1585,7 +1585,7 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo mounts := []corev1.VolumeMount{ { - Name: cr.Name, + Name: cr.Name + "-storage", MountPath: "/data", SubPath: "seaweed", }, @@ -1751,7 +1751,7 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC mounts := []corev1.VolumeMount{ { - Name: cr.Name, + Name: cr.Name + "-database", MountPath: "/data", SubPath: "postgres", }, @@ -1889,6 +1889,38 @@ func getPullPolicy(imageTag string) corev1.PullPolicy { return corev1.PullIfNotPresent } +func newVolumeForCR(cr *model.CryostatInstance) []corev1.Volume { + var volumeSource corev1.VolumeSource + if useEmptyDir(cr) { + emptyDir := cr.Spec.StorageOptions.EmptyDir + + sizeLimit, err := resource.ParseQuantity(emptyDir.SizeLimit) + if err != nil { + sizeLimit = *resource.NewQuantity(0, resource.BinarySI) + } + + volumeSource = corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: emptyDir.Medium, + SizeLimit: &sizeLimit, + }, + } + } else { + volumeSource = corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: cr.Name, + }, + } + } + + return []corev1.Volume{ + { + Name: cr.Name, + VolumeSource: volumeSource, + }, + } +} + func newVolumeForDatabse(cr *model.CryostatInstance) []corev1.Volume { var volumeSource corev1.VolumeSource if useEmptyDir(cr) { diff --git a/internal/controllers/pvc.go b/internal/controllers/pvc.go index bc1d2007..929d0f3f 100644 --- a/internal/controllers/pvc.go +++ b/internal/controllers/pvc.go @@ -31,6 +31,36 @@ import ( // Event type to inform users of invalid PVC specs const eventPersistentVolumeClaimInvalidType = "PersistentVolumeClaimInvalid" +func (r *Reconciler) reconcileCorePVC(ctx context.Context, cr *model.CryostatInstance) error { + emptyDir := cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.EmptyDir != nil && cr.Spec.StorageOptions.EmptyDir.Enabled + if emptyDir { + // If user requested an emptyDir volume, then do nothing. + // Don't delete the PVC to prevent accidental data loss + // depending on the reclaim policy. + return nil + } + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: cr.Name, + Namespace: cr.InstallNamespace, + }, + } + + // Look up PVC configuration, applying defaults where needed + config := configurePVC(cr) + + err := r.createOrUpdatePVC(ctx, pvc, cr.Object, config) + if err != nil { + // If the API server says the PVC is invalid, emit a warning event + // to inform the user. + if kerrors.IsInvalid(err) { + r.EventRecorder.Event(cr.Object, corev1.EventTypeWarning, eventPersistentVolumeClaimInvalidType, err.Error()) + } + return err + } + return nil +} + func (r *Reconciler) reconcileDatabasePVC(ctx context.Context, cr *model.CryostatInstance) error { emptyDir := cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.EmptyDir != nil && cr.Spec.StorageOptions.EmptyDir.Enabled if emptyDir { diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 6a01608f..3469d0ab 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -204,6 +204,11 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn return reconcile.Result{}, err } + err = r.reconcileCorePVC(ctx, cr) + if err != nil { + return reconcile.Result{}, err + } + err = r.reconcileSecrets(ctx, cr) if err != nil { return reconcile.Result{}, err @@ -400,7 +405,6 @@ func (r *Reconciler) reconcileDatabase(ctx context.Context, reqLogger logr.Logge } deployment := resources.NewDeploymentForDatabase(cr, imageTags, tls, r.IsOpenShift, fsGroup) - r.Log.Info(deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name) err = r.createOrUpdateDeployment(ctx, deployment, cr.Object) if err != nil { return reconcile.Result{}, err @@ -578,11 +582,9 @@ func (r *Reconciler) createOrUpdateDeployment(ctx context.Context, deploy *appsv return nil }) if err != nil { - r.Log.Info("sdfsfsdg err") if err == errSelectorModified { return r.recreateDeployment(ctx, deployCopy, owner) } - r.Log.Info(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name) return err } r.Log.Info(fmt.Sprintf("Deployment %s", op), "name", deploy.Name, "namespace", deploy.Namespace) diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 3375c9e6..37a09bf5 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -142,10 +142,13 @@ func resourceChecks() []resourceCheck { {(*cryostatTestInput).expectRBAC, "RBAC"}, {(*cryostatTestInput).expectRoutes, "routes"}, {func(t *cryostatTestInput) { - t.expectPVC(t.NewDefaultPVC(t.Name+"-database"), t.Name+"-database") + t.expectPVC(t.NewDefaultPVC(), t.Name) + }, "cryostat persistent volume claim"}, + {func(t *cryostatTestInput) { + t.expectPVC(t.NewDatabasePVC(), t.Name+"-database") }, "database persistent volume claim"}, {func(t *cryostatTestInput) { - t.expectPVC(t.NewDefaultPVC(t.Name+"-storage"), t.Name+"-storage") + t.expectPVC(t.NewStoragePVC(), t.Name+"-storage") }, "storage persistent volume claim"}, {(*cryostatTestInput).expectDatabaseSecret, "database secret"}, {(*cryostatTestInput).expectStorageSecret, "object storage secret"}, @@ -821,8 +824,7 @@ func (c *controllerTest) commonTests() { t.reconcileCryostatFully() }) It("should create the PVC with requested spec", func() { - t.expectPVC(t.NewCustomPVC(t.Name+"-database"), t.Name+"-database") - t.expectPVC(t.NewCustomPVC(t.Name+"-storage"), t.Name+"-storage") + t.expectPVC(t.NewCustomPVC(), t.Name) }) }) Context("with custom PVC spec overriding some defaults", func() { @@ -833,8 +835,7 @@ func (c *controllerTest) commonTests() { t.reconcileCryostatFully() }) It("should create the PVC with requested spec", func() { - t.expectPVC(t.NewCustomPVCSomeDefault(t.Name+"-database"), t.Name+"-database") - t.expectPVC(t.NewCustomPVCSomeDefault(t.Name+"-storage"), t.Name+"-storage") + t.expectPVC(t.NewCustomPVCSomeDefault(), t.Name) }) }) Context("with custom PVC config with no spec", func() { @@ -845,52 +846,35 @@ func (c *controllerTest) commonTests() { t.reconcileCryostatFully() }) It("should create the PVC with requested label", func() { - t.expectPVC(t.NewDefaultPVCWithLabel(t.Name+"-database"), t.Name+"-database") - t.expectPVC(t.NewDefaultPVCWithLabel(t.Name+"-storage"), t.Name+"-storage") + t.expectPVC(t.NewDefaultPVCWithLabel(), t.Name) }) }) Context("with an existing PVC", func() { - var oldDatabasePVC *corev1.PersistentVolumeClaim - var oldStoragePVC *corev1.PersistentVolumeClaim + var oldPVC *corev1.PersistentVolumeClaim BeforeEach(func() { - oldDatabasePVC = t.NewDefaultPVC(t.Name + "-database") - oldStoragePVC = t.NewDefaultPVC(t.Name + "-storage") - t.objs = append(t.objs, t.NewCryostatWithPVCSpec().Object, oldDatabasePVC, oldStoragePVC) + oldPVC = t.NewDefaultPVC() + t.objs = append(t.objs, t.NewCryostatWithPVCSpec().Object, oldPVC) }) Context("that successfully updates", func() { BeforeEach(func() { // Add some labels and annotations to test merging - metav1.SetMetaDataLabel(&oldDatabasePVC.ObjectMeta, "my", "other-label") - metav1.SetMetaDataLabel(&oldDatabasePVC.ObjectMeta, "another", "label") - metav1.SetMetaDataAnnotation(&oldDatabasePVC.ObjectMeta, "my/custom", "other-annotation") - metav1.SetMetaDataAnnotation(&oldDatabasePVC.ObjectMeta, "another/custom", "annotation") - - metav1.SetMetaDataLabel(&oldStoragePVC.ObjectMeta, "my", "other-label") - metav1.SetMetaDataLabel(&oldStoragePVC.ObjectMeta, "another", "label") - metav1.SetMetaDataAnnotation(&oldStoragePVC.ObjectMeta, "my/custom", "other-annotation") - metav1.SetMetaDataAnnotation(&oldStoragePVC.ObjectMeta, "another/custom", "annotation") + metav1.SetMetaDataLabel(&oldPVC.ObjectMeta, "my", "other-label") + metav1.SetMetaDataLabel(&oldPVC.ObjectMeta, "another", "label") + metav1.SetMetaDataAnnotation(&oldPVC.ObjectMeta, "my/custom", "other-annotation") + metav1.SetMetaDataAnnotation(&oldPVC.ObjectMeta, "another/custom", "annotation") }) JustBeforeEach(func() { t.reconcileCryostatFully() }) It("should update metadata and resource requests", func() { - expectedDatabase := t.NewDefaultPVC(t.Name + "-database") - expectedStorage := t.NewDefaultPVC(t.Name + "-storage") - metav1.SetMetaDataLabel(&expectedDatabase.ObjectMeta, "my", "label") - metav1.SetMetaDataLabel(&expectedDatabase.ObjectMeta, "another", "label") - metav1.SetMetaDataLabel(&expectedDatabase.ObjectMeta, "app", t.Name) - metav1.SetMetaDataAnnotation(&expectedDatabase.ObjectMeta, "my/custom", "annotation") - metav1.SetMetaDataAnnotation(&expectedDatabase.ObjectMeta, "another/custom", "annotation") - expectedDatabase.Spec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse("10Gi") - t.expectPVC(expectedDatabase, t.Name+"-database") - - metav1.SetMetaDataLabel(&expectedStorage.ObjectMeta, "my", "label") - metav1.SetMetaDataLabel(&expectedStorage.ObjectMeta, "another", "label") - metav1.SetMetaDataLabel(&expectedStorage.ObjectMeta, "app", t.Name) - metav1.SetMetaDataAnnotation(&expectedStorage.ObjectMeta, "my/custom", "annotation") - metav1.SetMetaDataAnnotation(&expectedStorage.ObjectMeta, "another/custom", "annotation") - expectedStorage.Spec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse("10Gi") - t.expectPVC(expectedStorage, t.Name+"-storage") + expected := t.NewDefaultPVC() + metav1.SetMetaDataLabel(&expected.ObjectMeta, "my", "label") + metav1.SetMetaDataLabel(&expected.ObjectMeta, "another", "label") + metav1.SetMetaDataLabel(&expected.ObjectMeta, "app", t.Name) + metav1.SetMetaDataAnnotation(&expected.ObjectMeta, "my/custom", "annotation") + metav1.SetMetaDataAnnotation(&expected.ObjectMeta, "another/custom", "annotation") + expected.Spec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse("10Gi") + t.expectPVC(expected, t.Name) }) }) /**Context("that fails to update", func() { diff --git a/internal/test/resources.go b/internal/test/resources.go index 2d08aad6..8c6f0e5d 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1194,7 +1194,7 @@ func (r *TestResources) newPVC(spec *corev1.PersistentVolumeClaimSpec, labels ma } } -func (r *TestResources) NewDefaultPVC(name string) *corev1.PersistentVolumeClaim { +func (r *TestResources) NewDefaultPVC() *corev1.PersistentVolumeClaim { return r.newPVC(&corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.ResourceRequirements{ @@ -1204,10 +1204,36 @@ func (r *TestResources) NewDefaultPVC(name string) *corev1.PersistentVolumeClaim }, }, map[string]string{ "app": r.Name, - }, nil, name) + }, nil, r.Name) } -func (r *TestResources) NewCustomPVC(name string) *corev1.PersistentVolumeClaim { +func (r *TestResources) NewDatabasePVC() *corev1.PersistentVolumeClaim { + return r.newPVC(&corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("500Mi"), + }, + }, + }, map[string]string{ + "app": r.Name, + }, nil, r.Name+"-database") +} + +func (r *TestResources) NewStoragePVC() *corev1.PersistentVolumeClaim { + return r.newPVC(&corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("500Mi"), + }, + }, + }, map[string]string{ + "app": r.Name, + }, nil, r.Name+"-storage") +} + +func (r *TestResources) NewCustomPVC() *corev1.PersistentVolumeClaim { storageClass := "cool-storage" return r.newPVC(&corev1.PersistentVolumeClaimSpec{ StorageClassName: &storageClass, @@ -1222,10 +1248,10 @@ func (r *TestResources) NewCustomPVC(name string) *corev1.PersistentVolumeClaim "app": r.Name, }, map[string]string{ "my/custom": "annotation", - }, name) + }, r.Name) } -func (r *TestResources) NewCustomPVCSomeDefault(name string) *corev1.PersistentVolumeClaim { +func (r *TestResources) NewCustomPVCSomeDefault() *corev1.PersistentVolumeClaim { storageClass := "" return r.newPVC(&corev1.PersistentVolumeClaimSpec{ StorageClassName: &storageClass, @@ -1237,10 +1263,10 @@ func (r *TestResources) NewCustomPVCSomeDefault(name string) *corev1.PersistentV }, }, map[string]string{ "app": r.Name, - }, nil, name) + }, nil, r.Name) } -func (r *TestResources) NewDefaultPVCWithLabel(name string) *corev1.PersistentVolumeClaim { +func (r *TestResources) NewDefaultPVCWithLabel() *corev1.PersistentVolumeClaim { return r.newPVC(&corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.ResourceRequirements{ @@ -1251,7 +1277,7 @@ func (r *TestResources) NewDefaultPVCWithLabel(name string) *corev1.PersistentVo }, map[string]string{ "app": r.Name, "my": "label", - }, nil, name) + }, nil, r.Name) } func (r *TestResources) NewDefaultEmptyDir() *corev1.EmptyDirVolumeSource { @@ -1946,7 +1972,7 @@ func (r *TestResources) NewStorageVolumeMounts() []corev1.VolumeMount { mounts := []corev1.VolumeMount{} mounts = append(mounts, corev1.VolumeMount{ - Name: r.Name, + Name: r.Name + "-storage", MountPath: "/data", SubPath: "seaweed", }) @@ -1965,7 +1991,7 @@ func (r *TestResources) NewDatabaseVolumeMounts() []corev1.VolumeMount { mounts := []corev1.VolumeMount{} mounts = append(mounts, corev1.VolumeMount{ - Name: r.Name, + Name: r.Name + "-database", MountPath: "/data", SubPath: "postgres", }) @@ -2352,7 +2378,17 @@ func (r *TestResources) NewAuthPropertiesVolume() corev1.Volume { func (r *TestResources) newVolumes(certProjections []corev1.VolumeProjection) []corev1.Volume { readOnlymode := int32(0440) - volumes := []corev1.Volume{} + volumes := []corev1.Volume{ + { + Name: r.Name, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: r.Name, + ReadOnly: false, + }, + }, + }, + } projs := append([]corev1.VolumeProjection{}, certProjections...) if r.TLS { projs = append(projs, corev1.VolumeProjection{ From f13ca566ed439dad70df4e0b096b854a002e6a9c Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Tue, 30 Jul 2024 10:44:08 -0400 Subject: [PATCH 07/17] tmp without tls --- ...yostat-operator.clusterserviceversion.yaml | 2 +- internal/controllers/certmanager.go | 8 +-- .../resource_definitions/certificates.go | 2 + .../resource_definitions.go | 51 ++++++++-------- internal/controllers/reconciler_test.go | 2 +- internal/test/clients.go | 2 +- internal/test/resources.go | 59 ++++++++++--------- 7 files changed, 66 insertions(+), 60 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 59fa2a9d..b0dbe3fa 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -35,7 +35,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-07-29T20:27:49Z" + createdAt: "2024-07-29T20:48:29Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/internal/controllers/certmanager.go b/internal/controllers/certmanager.go index 90b32e45..6f2446c5 100644 --- a/internal/controllers/certmanager.go +++ b/internal/controllers/certmanager.go @@ -90,6 +90,7 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) ( return nil, err } + /** // Create a certificate for the Cryostat database signed by the Cryostat CA databaseCert := resources.NewDatabaseCert(cr) err = r.createOrUpdateCertificate(ctx, databaseCert, cr.Object) @@ -102,15 +103,14 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) ( err = r.createOrUpdateCertificate(ctx, storageCert, cr.Object) if err != nil { return nil, err - } + }**/ + tlsConfig := &resources.TLSConfig{ CryostatSecret: cryostatCert.Spec.SecretName, ReportsSecret: reportsCert.Spec.SecretName, - DatabaseSecret: databaseCert.Spec.SecretName, - StorageSecret: storageCert.Spec.SecretName, KeystorePassSecret: cryostatCert.Spec.Keystores.PKCS12.PasswordSecretRef.Name, } - certificates := []*certv1.Certificate{caCert, cryostatCert, reportsCert, databaseCert, storageCert} + certificates := []*certv1.Certificate{caCert, cryostatCert, reportsCert} // Update owner references of TLS secrets created by cert-manager to ensure proper cleanup err = r.setCertSecretOwner(ctx, cr.Object, certificates...) diff --git a/internal/controllers/common/resource_definitions/certificates.go b/internal/controllers/common/resource_definitions/certificates.go index 5213eeea..31a745fc 100644 --- a/internal/controllers/common/resource_definitions/certificates.go +++ b/internal/controllers/common/resource_definitions/certificates.go @@ -132,6 +132,7 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate { } } +/** func NewDatabaseCert(cr *model.CryostatInstance) *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -179,3 +180,4 @@ func NewStorageCert(cr *model.CryostatInstance) *certv1.Certificate { }, } } +**/ diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 6c62a43d..37fdd770 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -62,9 +62,9 @@ type TLSConfig struct { // Name of the TLS secret for Reports Generator ReportsSecret string // Name of the TLS secret for Database - DatabaseSecret string + // DatabaseSecret string // Name of the TLS secret for Storage - StorageSecret string + // StorageSecret string // Name of the secret containing the password for the keystore in CryostatSecret KeystorePassSecret string // PEM-encoded X.509 certificate for the Cryostat CA @@ -618,6 +618,7 @@ func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TL container := []corev1.Container{NewDatabaseContainer(cr, imageTags.DatabaseImageTag, tls)} volumes := newVolumeForDatabse(cr) + /** if tls != nil { secretVolume := corev1.Volume{ Name: "database-tls-secret", @@ -628,7 +629,7 @@ func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TL }, } volumes = append(volumes, secretVolume) - } + }**/ var podSc *corev1.PodSecurityContext if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.PodSecurityContext != nil { @@ -674,6 +675,7 @@ func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLS container := []corev1.Container{NewStorageContainer(cr, imageTags.StorageImageTag, tls)} volumes := newVolumeForStorage(cr) + /** if tls != nil { secretVolume := corev1.Volume{ Name: "storage-tls-secret", @@ -684,7 +686,7 @@ func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLS }, } volumes = append(volumes, secretVolume) - } + }**/ var podSc *corev1.PodSecurityContext if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.PodSecurityContext != nil { @@ -1213,7 +1215,7 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag }, { Name: "QUARKUS_DATASOURCE_JDBC_URL", - Value: "jdbc:postgresql://localhost:5432/cryostat", + Value: fmt.Sprintf("jdbc:postgresql://%s-database:5432/cryostat", cr.Name), }, { Name: "STORAGE_BUCKETS_ARCHIVE_NAME", @@ -1221,7 +1223,7 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag }, { Name: "QUARKUS_S3_ENDPOINT_OVERRIDE", - Value: "http://localhost:8333", + Value: fmt.Sprintf("http://%s-storage:8333", cr.Name), }, { Name: "QUARKUS_S3_PATH_STYLE_ACCESS", @@ -1607,24 +1609,21 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo }) livenessProbeScheme := corev1.URISchemeHTTP + /** if tls != nil { tlsEnvs := []corev1.EnvVar{ { - Name: "QUARKUS_HTTP_SSL_PORT", + Name: "S3_PORT_HTTPS", Value: strconv.Itoa(int(constants.StorageContainerPort)), }, { - Name: "QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES", + Name: "S3_KEY_FILE", Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.StorageSecret, corev1.TLSPrivateKeyKey), }, { - Name: "QUARKUS_HTTP_SSL_CERTIFICATE_FILES", + Name: "S3_CERT_FILE", Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.StorageSecret, corev1.TLSCertKey), }, - { - Name: "QUARKUS_HTTP_INSECURE_REQUESTS", - Value: "disabled", - }, } tlsSecretMount := corev1.VolumeMount{ @@ -1641,7 +1640,7 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo Name: "QUARKUS_HTTP_PORT", Value: strconv.Itoa(int(constants.StorageContainerPort)), }) - } + }**/ if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.StorageSecurityContext != nil { containerSc = cr.Spec.SecurityOptions.StorageSecurityContext @@ -1757,26 +1756,26 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC }, } + /** if tls != nil { tlsEnvs := []corev1.EnvVar{ { - Name: "QUARKUS_HTTP_SSL_PORT", - Value: strconv.Itoa(int(constants.DatabaseContainerPort)), + Name: "QUARKUS_DATASOURCE_REACTIVE_TRUST_ALL", + Value: "true", }, { - Name: "QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES", - Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.DatabaseSecret, corev1.TLSPrivateKeyKey), + Name: "QUARKUS_DATASOURCE_REACTIVE_KEY_CERTIFICATE_PEM_KEYS", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls/tls.key", cr.Name), }, { - Name: "QUARKUS_HTTP_SSL_CERTIFICATE_FILES", - Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.DatabaseSecret, corev1.TLSCertKey), + Name: "QUARKUS_DATASOURCE_REACTIVE_KEY_CERTIFICATE_PEM_CERTS", + Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls/tls.crt", cr.Name), }, { - Name: "QUARKUS_HTTP_INSECURE_REQUESTS", - Value: "disabled", + Name: "QUARKUS_DATASOURCE_REACTIVE_URL", + Value: fmt.Sprintf("https://%s-database:5432", cr.Name), }, } - tlsSecretMount := corev1.VolumeMount{ Name: "database-tls-secret", MountPath: "/var/run/secrets/operator.cryostat.io/" + tls.DatabaseSecret, @@ -1787,10 +1786,10 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC mounts = append(mounts, tlsSecretMount) } else { envs = append(envs, corev1.EnvVar{ - Name: "QUARKUS_HTTP_PORT", - Value: strconv.Itoa(int(constants.DatabaseContainerPort)), + Name: "QUARKUS_DATASOURCE_REACTIVE_URL", + Value: fmt.Sprintf("http://%s-database:5432", cr.Name), }) - } + }**/ return corev1.Container{ Name: cr.Name + "-db", diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 37a09bf5..f819aa0b 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -2289,7 +2289,7 @@ func (t *cryostatTestInput) expectWaitingForCertificate() { func (t *cryostatTestInput) expectCertificates() { // Check certificates - certs := []*certv1.Certificate{t.NewCryostatCert(), t.NewCACert(), t.NewReportsCert(), t.NewDatabaseCert(), t.NewStorageCert()} + certs := []*certv1.Certificate{t.NewCryostatCert(), t.NewCACert(), t.NewReportsCert()} for _, expected := range certs { actual := &certv1.Certificate{} err := t.Client.Get(context.Background(), types.NamespacedName{Name: expected.Name, Namespace: expected.Namespace}, actual) diff --git a/internal/test/clients.go b/internal/test/clients.go index 4fa92c4c..4a46133e 100644 --- a/internal/test/clients.go +++ b/internal/test/clients.go @@ -70,7 +70,7 @@ func (c *testClient) makeCertificatesReady(ctx context.Context, obj runtime.Obje // If this object is one of the operator-managed certificates, mock the behaviour // of cert-manager processing those certificates cert, ok := obj.(*certv1.Certificate) - if ok && c.matchesName(cert, c.NewCryostatCert(), c.NewCACert(), c.NewReportsCert(), c.NewDatabaseCert(), c.NewStorageCert()) && + if ok && c.matchesName(cert, c.NewCryostatCert(), c.NewCACert(), c.NewReportsCert()) && len(cert.Status.Conditions) == 0 { // Create certificate secret c.createCertSecret(ctx, cert) diff --git a/internal/test/resources.go b/internal/test/resources.go index 8c6f0e5d..d3a37e97 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1066,6 +1066,7 @@ func (r *TestResources) NewReportsCert() *certv1.Certificate { } } +/** func (r *TestResources) NewDatabaseCert() *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1116,7 +1117,7 @@ func (r *TestResources) NewStorageCert() *certv1.Certificate { }, }, } -} +}**/ func (r *TestResources) NewCACert() *certv1.Certificate { return &certv1.Certificate{ @@ -1388,7 +1389,7 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, databaseU }, { Name: "QUARKUS_DATASOURCE_JDBC_URL", - Value: "jdbc:postgresql://localhost:5432/cryostat", + Value: fmt.Sprintf("jdbc:postgresql://%s-database:5432/cryostat", r.Name), }, { Name: "STORAGE_BUCKETS_ARCHIVE_NAME", @@ -1396,7 +1397,7 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, databaseU }, { Name: "QUARKUS_S3_ENDPOINT_OVERRIDE", - Value: "http://localhost:8333", + Value: fmt.Sprintf("http://%s-storage:8333", r.Name), }, { Name: "QUARKUS_S3_PATH_STYLE_ACCESS", @@ -1654,26 +1655,24 @@ func (r *TestResources) NewStorageEnvironmentVariables() []corev1.EnvVar { }, }, } + /** if r.TLS { envs = append(envs, corev1.EnvVar{ - Name: "QUARKUS_HTTP_SSL_PORT", + Name: "S3_PORT_HTTPS", Value: "8333", }, corev1.EnvVar{ - Name: "QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES", + Name: "S3_KEY_FILE", Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls/tls.key", r.Name), }, corev1.EnvVar{ - Name: "QUARKUS_HTTP_SSL_CERTIFICATE_FILES", + Name: "S3_CERT_FILE", Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls/tls.crt", r.Name), - }, corev1.EnvVar{ - Name: "QUARKUS_HTTP_INSECURE_REQUESTS", - Value: "disabled", }) } else { envs = append(envs, corev1.EnvVar{ Name: "QUARKUS_HTTP_PORT", Value: "8333", }) - } + }**/ return envs } @@ -1717,26 +1716,27 @@ func (r *TestResources) NewDatabaseEnvironmentVariables(dbSecretProvided bool) [ }, }, } + /** if r.TLS { envs = append(envs, corev1.EnvVar{ - Name: "QUARKUS_HTTP_SSL_PORT", - Value: "5432", + Name: "QUARKUS_DATASOURCE_REACTIVE_TRUST_ALL", + Value: "true", }, corev1.EnvVar{ - Name: "QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES", + Name: "QUARKUS_DATASOURCE_REACTIVE_KEY_CERTIFICATE_PEM_KEYS", Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls/tls.key", r.Name), }, corev1.EnvVar{ - Name: "QUARKUS_HTTP_SSL_CERTIFICATE_FILES", + Name: "QUARKUS_DATASOURCE_REACTIVE_KEY_CERTIFICATE_PEM_CERTS", Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls/tls.crt", r.Name), }, corev1.EnvVar{ - Name: "QUARKUS_HTTP_INSECURE_REQUESTS", - Value: "disabled", + Name: "QUARKUS_DATASOURCE_REACTIVE_URL", + Value: fmt.Sprintf("https://%s-database:5432", r.Name), }) } else { envs = append(envs, corev1.EnvVar{ - Name: "QUARKUS_HTTP_PORT", - Value: "5432", + Name: "QUARKUS_DATASOURCE_REACTIVE_URL", + Value: fmt.Sprintf("http://%s-database:5432", r.Name), }) - } + }**/ return envs } @@ -1976,6 +1976,7 @@ func (r *TestResources) NewStorageVolumeMounts() []corev1.VolumeMount { MountPath: "/data", SubPath: "seaweed", }) + /** if r.TLS { mounts = append(mounts, corev1.VolumeMount{ @@ -1983,7 +1984,7 @@ func (r *TestResources) NewStorageVolumeMounts() []corev1.VolumeMount { MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls", r.Name), ReadOnly: true, }) - } + }**/ return mounts } @@ -1995,6 +1996,7 @@ func (r *TestResources) NewDatabaseVolumeMounts() []corev1.VolumeMount { MountPath: "/data", SubPath: "postgres", }) + /** if r.TLS { mounts = append(mounts, corev1.VolumeMount{ @@ -2002,7 +2004,7 @@ func (r *TestResources) NewDatabaseVolumeMounts() []corev1.VolumeMount { MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls", r.Name), ReadOnly: true, }) - } + }**/ return mounts } @@ -2127,10 +2129,11 @@ func (r *TestResources) NewDatasourceLivenessProbe() *corev1.Probe { } func (r *TestResources) NewStorageLivenessProbe() *corev1.Probe { - protocol := corev1.URISchemeHTTPS - if !r.TLS { - protocol = corev1.URISchemeHTTP - } + protocol := corev1.URISchemeHTTP + /** + if r.TLS { + protocol = corev1.URISchemeHTTPS + }**/ return &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ @@ -2496,6 +2499,7 @@ func (r *TestResources) NewDatabaseVolumes() []corev1.Volume { }, } + /** if r.TLS { volumes = append(volumes, corev1.Volume{ Name: "database-tls-secret", @@ -2505,7 +2509,7 @@ func (r *TestResources) NewDatabaseVolumes() []corev1.Volume { }, }, }) - } + } **/ return volumes } @@ -2522,6 +2526,7 @@ func (r *TestResources) NewStorageVolumes() []corev1.Volume { }, } + /** if r.TLS { volumes = append(volumes, corev1.Volume{ Name: "storage-tls-secret", @@ -2531,7 +2536,7 @@ func (r *TestResources) NewStorageVolumes() []corev1.Volume { }, }, }) - } + }**/ return volumes } From c1581ea26bc03c93078cf1fdcdc9b7e15db81e04 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Tue, 30 Jul 2024 15:04:44 -0400 Subject: [PATCH 08/17] tmp --- ...yostat-operator.clusterserviceversion.yaml | 2 +- .../resource_definitions.go | 15 +++--------- internal/controllers/reconciler_test.go | 23 ++---------------- internal/test/resources.go | 24 ++++--------------- 4 files changed, 10 insertions(+), 54 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index b0dbe3fa..8dd09edc 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -35,7 +35,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-07-29T20:48:29Z" + createdAt: "2024-07-30T14:31:04Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 37fdd770..67f61e72 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -929,7 +929,7 @@ func NewOpenShiftAuthProxyContainer(cr *model.CryostatInstance, specs *ServiceSp "--pass-basic-auth=false", fmt.Sprintf("--upstream=http://localhost:%d/", constants.CryostatHTTPContainerPort), fmt.Sprintf("--upstream=http://localhost:%d/grafana/", constants.GrafanaContainerPort), - fmt.Sprintf("--upstream=http://localhost:%d/storage/", constants.StorageContainerPort), + // fmt.Sprintf("--upstream=http://localhost:%d/storage/", constants.StorageContainerPort), fmt.Sprintf("--openshift-service-account=%s", cr.Name), "--proxy-websockets=true", "--proxy-prefix=/oauth2", @@ -1215,7 +1215,7 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag }, { Name: "QUARKUS_DATASOURCE_JDBC_URL", - Value: fmt.Sprintf("jdbc:postgresql://%s-database:5432/cryostat", cr.Name), + Value: fmt.Sprintf("jdbc:postgresql://%s-database.%s.svc.cluster.local:5432/cryostat", cr.Name, cr.InstallNamespace), }, { Name: "STORAGE_BUCKETS_ARCHIVE_NAME", @@ -1223,7 +1223,7 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag }, { Name: "QUARKUS_S3_ENDPOINT_OVERRIDE", - Value: fmt.Sprintf("http://%s-storage:8333", cr.Name), + Value: fmt.Sprintf("http://%s-storage.%s.svc.cluster.local:8333", cr.Name, cr.InstallNamespace), }, { Name: "QUARKUS_S3_PATH_STYLE_ACCESS", @@ -1328,15 +1328,6 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag envs = append(envs, reportsEnvs...) } - envs = append(envs, corev1.EnvVar{ - Name: "CRYOSTAT_SERVICES_DATABASE_URL", - Value: specs.DatabaseURL.String(), - }) - envs = append(envs, corev1.EnvVar{ - Name: "CRYOSTAT_SERVICES_STORAGE_URL", - Value: specs.StorageURL.String(), - }) - // Define INSIGHTS_PROXY URL if Insights integration is enabled if specs.InsightsURL != nil { insightsEnvs := []corev1.EnvVar{ diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index f819aa0b..81b7e6bf 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -2680,22 +2680,12 @@ func (t *cryostatTestInput) checkMainPodTemplate(deployment *appsv1.Deployment, // Check that the networking environment variables are set correctly coreContainer := template.Spec.Containers[0] reportPort := int32(10000) - databasePort := int32(5432) - storagePort := int32(8333) if cr.Spec.ServiceOptions != nil { if cr.Spec.ServiceOptions.ReportsConfig != nil && cr.Spec.ServiceOptions.ReportsConfig.HTTPPort != nil { reportPort = *cr.Spec.ServiceOptions.ReportsConfig.HTTPPort } - if cr.Spec.ServiceOptions.DatabaseConfig != nil && cr.Spec.ServiceOptions.DatabaseConfig.HTTPPort != nil { - databasePort = *cr.Spec.ServiceOptions.DatabaseConfig.HTTPPort - } - if cr.Spec.ServiceOptions.StorageConfig != nil && cr.Spec.ServiceOptions.StorageConfig.HTTPPort != nil { - storagePort = *cr.Spec.ServiceOptions.StorageConfig.HTTPPort - } } var reportsUrl string - var databaseUrl string - var storageUrl string if t.ReportReplicas == 0 { reportsUrl = "" } else if t.TLS { @@ -2703,13 +2693,6 @@ func (t *cryostatTestInput) checkMainPodTemplate(deployment *appsv1.Deployment, } else { reportsUrl = fmt.Sprintf("http://%s-reports:%d", t.Name, reportPort) } - if t.TLS { - databaseUrl = fmt.Sprintf("https://%s-database:%d", t.Name, databasePort) - storageUrl = fmt.Sprintf("https://%s-storage:%d", t.Name, storagePort) - } else { - databaseUrl = fmt.Sprintf("http://%s-database:%d", t.Name, databasePort) - storageUrl = fmt.Sprintf("http://%s-storage:%d", t.Name, storagePort) - } ingress := !t.OpenShift && cr.Spec.NetworkOptions != nil && cr.Spec.NetworkOptions.CoreConfig != nil && cr.Spec.NetworkOptions.CoreConfig.IngressSpec != nil emptyDir := cr.Spec.StorageOptions != nil && cr.Spec.StorageOptions.EmptyDir != nil && cr.Spec.StorageOptions.EmptyDir.Enabled @@ -2722,7 +2705,7 @@ func (t *cryostatTestInput) checkMainPodTemplate(deployment *appsv1.Deployment, cr.Spec.TargetDiscoveryOptions.DisableBuiltInPortNumbers dbSecretProvided := cr.Spec.DatabaseOptions != nil && cr.Spec.DatabaseOptions.SecretName != nil - t.checkCoreContainer(&coreContainer, ingress, reportsUrl, databaseUrl, storageUrl, + t.checkCoreContainer(&coreContainer, ingress, reportsUrl, emptyDir, hasPortConfig, builtInDiscoveryDisabled, @@ -2978,8 +2961,6 @@ func (t *cryostatTestInput) checkDeploymentHasTemplates() { func (t *cryostatTestInput) checkCoreContainer(container *corev1.Container, ingress bool, reportsUrl string, - databaseUrl string, - storageUrl string, emptyDir bool, hasPortConfig bool, builtInDiscoveryDisabled bool, builtInPortConfigDisabled bool, dbSecretProvided bool, @@ -2992,7 +2973,7 @@ func (t *cryostatTestInput) checkCoreContainer(container *corev1.Container, ingr Expect(container.Image).To(Equal(*t.EnvCoreImageTag)) } Expect(container.Ports).To(ConsistOf(t.NewCorePorts())) - Expect(container.Env).To(ConsistOf(t.NewCoreEnvironmentVariables(reportsUrl, databaseUrl, storageUrl, ingress, emptyDir, hasPortConfig, builtInDiscoveryDisabled, builtInPortConfigDisabled, dbSecretProvided))) + Expect(container.Env).To(ConsistOf(t.NewCoreEnvironmentVariables(reportsUrl, ingress, emptyDir, hasPortConfig, builtInDiscoveryDisabled, builtInPortConfigDisabled, dbSecretProvided))) Expect(container.EnvFrom).To(ConsistOf(t.NewCoreEnvFromSource())) Expect(container.VolumeMounts).To(ConsistOf(t.NewCoreVolumeMounts())) Expect(container.LivenessProbe).To(Equal(t.NewCoreLivenessProbe())) diff --git a/internal/test/resources.go b/internal/test/resources.go index d3a37e97..f051364c 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1352,7 +1352,7 @@ func (r *TestResources) NewAuthProxyPorts() []corev1.ContainerPort { } } -func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, databaseUrl string, storageUrl string, ingress bool, +func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, ingress bool, emptyDir bool, hasPortConfig bool, builtInDiscoveryDisabled bool, builtInPortConfigDisabled bool, dbSecretProvided bool) []corev1.EnvVar { envs := []corev1.EnvVar{ { @@ -1389,7 +1389,7 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, databaseU }, { Name: "QUARKUS_DATASOURCE_JDBC_URL", - Value: fmt.Sprintf("jdbc:postgresql://%s-database:5432/cryostat", r.Name), + Value: fmt.Sprintf("jdbc:postgresql://%s-database.%s.svc.cluster.local:5432/cryostat", r.Name, r.Namespace), }, { Name: "STORAGE_BUCKETS_ARCHIVE_NAME", @@ -1397,7 +1397,7 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, databaseU }, { Name: "QUARKUS_S3_ENDPOINT_OVERRIDE", - Value: fmt.Sprintf("http://%s-storage:8333", r.Name), + Value: fmt.Sprintf("http://%s-storage.%s.svc.cluster.local:8333", r.Name, r.Namespace), }, { Name: "QUARKUS_S3_PATH_STYLE_ACCESS", @@ -1501,22 +1501,6 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, databaseU }) } - if databaseUrl != "" { - envs = append(envs, - corev1.EnvVar{ - Name: "CRYOSTAT_SERVICES_DATABASE_URL", - Value: databaseUrl, - }) - } - - if storageUrl != "" { - envs = append(envs, - corev1.EnvVar{ - Name: "CRYOSTAT_SERVICES_STORAGE_URL", - Value: storageUrl, - }) - } - if len(r.InsightsURL) > 0 { envs = append(envs, corev1.EnvVar{ @@ -1904,7 +1888,7 @@ func (r *TestResources) NewAuthProxyArguments(authOptions *operatorv1beta2.Autho "--pass-basic-auth=false", "--upstream=http://localhost:8181/", "--upstream=http://localhost:3000/grafana/", - "--upstream=http://localhost:8333/storage/", + // "--upstream=http://localhost:8333/storage/", fmt.Sprintf("--openshift-service-account=%s", r.Name), "--proxy-websockets=true", "--proxy-prefix=/oauth2", From 9163ac7de1264a93958b4aa7f321d3e7d95f2070 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Wed, 31 Jul 2024 16:59:57 -0400 Subject: [PATCH 09/17] add db and storage certs --- ...yostat-operator.clusterserviceversion.yaml | 2 +- internal/controllers/certmanager.go | 5 ++-- .../resource_definitions/certificates.go | 2 -- .../resource_definitions.go | 28 ++++++++++--------- internal/test/resources.go | 20 ++++++------- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 8dd09edc..d99827ec 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -35,7 +35,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-07-30T14:31:04Z" + createdAt: "2024-07-30T18:48:36Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/internal/controllers/certmanager.go b/internal/controllers/certmanager.go index 6f2446c5..eff68fd4 100644 --- a/internal/controllers/certmanager.go +++ b/internal/controllers/certmanager.go @@ -90,7 +90,6 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) ( return nil, err } - /** // Create a certificate for the Cryostat database signed by the Cryostat CA databaseCert := resources.NewDatabaseCert(cr) err = r.createOrUpdateCertificate(ctx, databaseCert, cr.Object) @@ -103,10 +102,12 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) ( err = r.createOrUpdateCertificate(ctx, storageCert, cr.Object) if err != nil { return nil, err - }**/ + } tlsConfig := &resources.TLSConfig{ CryostatSecret: cryostatCert.Spec.SecretName, + DatabaseSecret: databaseCert.Spec.SecretName, + StorageSecret: storageCert.Spec.SecretName, ReportsSecret: reportsCert.Spec.SecretName, KeystorePassSecret: cryostatCert.Spec.Keystores.PKCS12.PasswordSecretRef.Name, } diff --git a/internal/controllers/common/resource_definitions/certificates.go b/internal/controllers/common/resource_definitions/certificates.go index 31a745fc..5213eeea 100644 --- a/internal/controllers/common/resource_definitions/certificates.go +++ b/internal/controllers/common/resource_definitions/certificates.go @@ -132,7 +132,6 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate { } } -/** func NewDatabaseCert(cr *model.CryostatInstance) *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -180,4 +179,3 @@ func NewStorageCert(cr *model.CryostatInstance) *certv1.Certificate { }, } } -**/ diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index 67f61e72..92fc5c0b 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -62,9 +62,9 @@ type TLSConfig struct { // Name of the TLS secret for Reports Generator ReportsSecret string // Name of the TLS secret for Database - // DatabaseSecret string + DatabaseSecret string // Name of the TLS secret for Storage - // StorageSecret string + StorageSecret string // Name of the secret containing the password for the keystore in CryostatSecret KeystorePassSecret string // PEM-encoded X.509 certificate for the Cryostat CA @@ -618,7 +618,7 @@ func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TL container := []corev1.Container{NewDatabaseContainer(cr, imageTags.DatabaseImageTag, tls)} volumes := newVolumeForDatabse(cr) - /** + if tls != nil { secretVolume := corev1.Volume{ Name: "database-tls-secret", @@ -629,7 +629,7 @@ func NewPodForDatabase(cr *model.CryostatInstance, imageTags *ImageTags, tls *TL }, } volumes = append(volumes, secretVolume) - }**/ + } var podSc *corev1.PodSecurityContext if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.PodSecurityContext != nil { @@ -675,7 +675,7 @@ func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLS container := []corev1.Container{NewStorageContainer(cr, imageTags.StorageImageTag, tls)} volumes := newVolumeForStorage(cr) - /** + if tls != nil { secretVolume := corev1.Volume{ Name: "storage-tls-secret", @@ -686,7 +686,7 @@ func NewPodForStorage(cr *model.CryostatInstance, imageTags *ImageTags, tls *TLS }, } volumes = append(volumes, secretVolume) - }**/ + } var podSc *corev1.PodSecurityContext if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.PodSecurityContext != nil { @@ -1223,7 +1223,7 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag }, { Name: "QUARKUS_S3_ENDPOINT_OVERRIDE", - Value: fmt.Sprintf("http://%s-storage.%s.svc.cluster.local:8333", cr.Name, cr.InstallNamespace), + Value: fmt.Sprintf("https://%s-storage.%s.svc.cluster.local:8333", cr.Name, cr.InstallNamespace), }, { Name: "QUARKUS_S3_PATH_STYLE_ACCESS", @@ -1600,8 +1600,9 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo }) livenessProbeScheme := corev1.URISchemeHTTP - /** + if tls != nil { + /** tlsEnvs := []corev1.EnvVar{ { Name: "S3_PORT_HTTPS", @@ -1616,6 +1617,7 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.StorageSecret, corev1.TLSCertKey), }, } + envs = append(envs, tlsEnvs...) **/ tlsSecretMount := corev1.VolumeMount{ Name: "storage-tls-secret", @@ -1623,10 +1625,9 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo ReadOnly: true, } - envs = append(envs, tlsEnvs...) mounts = append(mounts, tlsSecretMount) livenessProbeScheme = corev1.URISchemeHTTPS - } else { + } /** else { envs = append(envs, corev1.EnvVar{ Name: "QUARKUS_HTTP_PORT", Value: strconv.Itoa(int(constants.StorageContainerPort)), @@ -1747,8 +1748,8 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC }, } - /** if tls != nil { + /** tlsEnvs := []corev1.EnvVar{ { Name: "QUARKUS_DATASOURCE_REACTIVE_TRUST_ALL", @@ -1767,15 +1768,16 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC Value: fmt.Sprintf("https://%s-database:5432", cr.Name), }, } + envs = append(envs, tlsEnvs...) **/ + tlsSecretMount := corev1.VolumeMount{ Name: "database-tls-secret", MountPath: "/var/run/secrets/operator.cryostat.io/" + tls.DatabaseSecret, ReadOnly: true, } - envs = append(envs, tlsEnvs...) mounts = append(mounts, tlsSecretMount) - } else { + } /** else { envs = append(envs, corev1.EnvVar{ Name: "QUARKUS_DATASOURCE_REACTIVE_URL", Value: fmt.Sprintf("http://%s-database:5432", cr.Name), diff --git a/internal/test/resources.go b/internal/test/resources.go index f051364c..48178227 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1397,7 +1397,7 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, ingress b }, { Name: "QUARKUS_S3_ENDPOINT_OVERRIDE", - Value: fmt.Sprintf("http://%s-storage.%s.svc.cluster.local:8333", r.Name, r.Namespace), + Value: fmt.Sprintf("https://%s-storage.%s.svc.cluster.local:8333", r.Name, r.Namespace), }, { Name: "QUARKUS_S3_PATH_STYLE_ACCESS", @@ -1960,7 +1960,7 @@ func (r *TestResources) NewStorageVolumeMounts() []corev1.VolumeMount { MountPath: "/data", SubPath: "seaweed", }) - /** + if r.TLS { mounts = append(mounts, corev1.VolumeMount{ @@ -1968,7 +1968,7 @@ func (r *TestResources) NewStorageVolumeMounts() []corev1.VolumeMount { MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls", r.Name), ReadOnly: true, }) - }**/ + } return mounts } @@ -1980,7 +1980,7 @@ func (r *TestResources) NewDatabaseVolumeMounts() []corev1.VolumeMount { MountPath: "/data", SubPath: "postgres", }) - /** + if r.TLS { mounts = append(mounts, corev1.VolumeMount{ @@ -1988,7 +1988,7 @@ func (r *TestResources) NewDatabaseVolumeMounts() []corev1.VolumeMount { MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls", r.Name), ReadOnly: true, }) - }**/ + } return mounts } @@ -2114,10 +2114,10 @@ func (r *TestResources) NewDatasourceLivenessProbe() *corev1.Probe { func (r *TestResources) NewStorageLivenessProbe() *corev1.Probe { protocol := corev1.URISchemeHTTP - /** + if r.TLS { protocol = corev1.URISchemeHTTPS - }**/ + } return &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ @@ -2483,7 +2483,6 @@ func (r *TestResources) NewDatabaseVolumes() []corev1.Volume { }, } - /** if r.TLS { volumes = append(volumes, corev1.Volume{ Name: "database-tls-secret", @@ -2493,7 +2492,7 @@ func (r *TestResources) NewDatabaseVolumes() []corev1.Volume { }, }, }) - } **/ + } return volumes } @@ -2510,7 +2509,6 @@ func (r *TestResources) NewStorageVolumes() []corev1.Volume { }, } - /** if r.TLS { volumes = append(volumes, corev1.Volume{ Name: "storage-tls-secret", @@ -2520,7 +2518,7 @@ func (r *TestResources) NewStorageVolumes() []corev1.Volume { }, }, }) - }**/ + } return volumes } From 28efdea5c73d2f70e011bd83b43079ff5c26654c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 1 Nov 2024 10:20:32 -0400 Subject: [PATCH 10/17] comment out incomplete/incorrect db/storage tls configs --- .../resource_definitions.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index ecca863d..da1316fd 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -1639,8 +1639,8 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo livenessProbeScheme := corev1.URISchemeHTTP + /** if tls != nil { - /** tlsEnvs := []corev1.EnvVar{ { Name: "S3_PORT_HTTPS", @@ -1655,7 +1655,7 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo Value: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s/%s", tls.StorageSecret, corev1.TLSCertKey), }, } - envs = append(envs, tlsEnvs...) **/ + envs = append(envs, tlsEnvs...) tlsSecretMount := corev1.VolumeMount{ Name: "storage-tls-secret", @@ -1665,12 +1665,13 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo mounts = append(mounts, tlsSecretMount) livenessProbeScheme = corev1.URISchemeHTTPS - } /** else { + } else { envs = append(envs, corev1.EnvVar{ Name: "QUARKUS_HTTP_PORT", Value: strconv.Itoa(int(constants.StoragePort)), }) - }**/ + } + **/ if cr.Spec.SecurityOptions != nil && cr.Spec.SecurityOptions.StorageSecurityContext != nil { containerSc = cr.Spec.SecurityOptions.StorageSecurityContext @@ -1786,8 +1787,8 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC }, } + /** if tls != nil { - /** tlsEnvs := []corev1.EnvVar{ { Name: "QUARKUS_DATASOURCE_REACTIVE_TRUST_ALL", @@ -1806,7 +1807,7 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC Value: fmt.Sprintf("https://%s-database:5432", cr.Name), }, } - envs = append(envs, tlsEnvs...) **/ + envs = append(envs, tlsEnvs...) tlsSecretMount := corev1.VolumeMount{ Name: "database-tls-secret", @@ -1815,12 +1816,13 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC } mounts = append(mounts, tlsSecretMount) - } /** else { + } else { envs = append(envs, corev1.EnvVar{ Name: "QUARKUS_DATASOURCE_REACTIVE_URL", Value: fmt.Sprintf("http://%s-database:5432", cr.Name), }) - }**/ + } + **/ return corev1.Container{ Name: cr.Name + "-db", From d4c0035181080c473462f291a375fe1836ce7b30 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 1 Nov 2024 10:24:24 -0400 Subject: [PATCH 11/17] remove authproxy uid override --- config/samples/operator_v1beta2_cryostat.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/config/samples/operator_v1beta2_cryostat.yaml b/config/samples/operator_v1beta2_cryostat.yaml index d1823219..8f014f31 100644 --- a/config/samples/operator_v1beta2_cryostat.yaml +++ b/config/samples/operator_v1beta2_cryostat.yaml @@ -13,6 +13,3 @@ spec: spec: {} reportOptions: replicas: 0 - securityOptions: - authProxySecurityContext: - runAsUser: 1001 From 67cd857624bd33a715dfb68d31f0976f6f46dba5 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 1 Nov 2024 10:37:16 -0400 Subject: [PATCH 12/17] cryostat expects to talk to storage over http only for now --- .../common/resource_definitions/resource_definitions.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/controllers/common/resource_definitions/resource_definitions.go b/internal/controllers/common/resource_definitions/resource_definitions.go index da1316fd..349f132d 100644 --- a/internal/controllers/common/resource_definitions/resource_definitions.go +++ b/internal/controllers/common/resource_definitions/resource_definitions.go @@ -1210,6 +1210,11 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag configPath := "/opt/cryostat.d/conf.d" templatesPath := "/opt/cryostat.d/templates.d" + storageProtocol := "http" + // TODO + // if tls != nil { + // storageProtocol = "https" + // } envs := []corev1.EnvVar{ { Name: "QUARKUS_HTTP_HOST", @@ -1257,7 +1262,7 @@ func NewCoreContainer(cr *model.CryostatInstance, specs *ServiceSpecs, imageTag }, { Name: "QUARKUS_S3_ENDPOINT_OVERRIDE", - Value: fmt.Sprintf("https://%s-storage.%s.svc.cluster.local:8333", cr.Name, cr.InstallNamespace), + Value: fmt.Sprintf("%s://%s-storage.%s.svc.cluster.local:8333", storageProtocol, cr.Name, cr.InstallNamespace), }, { Name: "QUARKUS_S3_PATH_STYLE_ACCESS", @@ -1639,6 +1644,7 @@ func NewStorageContainer(cr *model.CryostatInstance, imageTag string, tls *TLSCo livenessProbeScheme := corev1.URISchemeHTTP + // TODO /** if tls != nil { tlsEnvs := []corev1.EnvVar{ @@ -1787,6 +1793,7 @@ func NewDatabaseContainer(cr *model.CryostatInstance, imageTag string, tls *TLSC }, } + // TODO /** if tls != nil { tlsEnvs := []corev1.EnvVar{ From f19babf5a5cf363ffb21efb243b622b5fe65ee37 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 1 Nov 2024 10:48:42 -0400 Subject: [PATCH 13/17] correct envtests --- internal/controllers/reconciler_test.go | 4 +-- internal/test/resources.go | 47 ++++++++++++++----------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 8ed16edc..3e1f14bf 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -3067,7 +3067,7 @@ func (t *cryostatTestInput) checkMainPodTemplate(deployment *appsv1.Deployment, Expect(template.Spec.SecurityContext).To(Equal(t.NewPodSecurityContext(cr))) // Check that the networking environment variables are set correctly - Expect(len(template.Spec.Containers)).To(Equal(7)) + Expect(len(template.Spec.Containers)).To(Equal(5)) coreContainer := template.Spec.Containers[0] reportPort := int32(10000) if cr.Spec.ServiceOptions != nil { @@ -3116,7 +3116,7 @@ func (t *cryostatTestInput) checkMainPodTemplate(deployment *appsv1.Deployment, t.checkAuthProxyContainer(&authProxyContainer, t.NewAuthProxyContainerResource(cr), t.NewAuthProxySecurityContext(cr), cr.Spec.AuthorizationOptions) // Check that Agent Proxy is configured properly - agentProxyContainer := template.Spec.Containers[6] + agentProxyContainer := template.Spec.Containers[4] t.checkAgentProxyContainer(&agentProxyContainer, t.NewAgentProxyContainerResource(cr), t.NewAgentProxySecurityContext(cr)) // Check that the proper Service Account is set diff --git a/internal/test/resources.go b/internal/test/resources.go index 669a3631..dea41e0b 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1582,6 +1582,11 @@ func (r *TestResources) NewAgentProxyPorts() []corev1.ContainerPort { func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, ingress bool, emptyDir bool, hasPortConfig bool, builtInDiscoveryDisabled bool, builtInPortConfigDisabled bool, dbSecretProvided bool) []corev1.EnvVar { + storageProtocol := "http" + // TODO + // if r.TLS { + // storageProtocol = "https" + // } envs := []corev1.EnvVar{ { Name: "QUARKUS_HTTP_HOST", @@ -1629,7 +1634,7 @@ func (r *TestResources) NewCoreEnvironmentVariables(reportsUrl string, ingress b }, { Name: "QUARKUS_S3_ENDPOINT_OVERRIDE", - Value: fmt.Sprintf("https://%s-storage.%s.svc.cluster.local:8333", r.Name, r.Namespace), + Value: fmt.Sprintf("%s://%s-storage.%s.svc.cluster.local:8333", storageProtocol, r.Name, r.Namespace), }, { Name: "QUARKUS_S3_PATH_STYLE_ACCESS", @@ -2211,14 +2216,15 @@ func (r *TestResources) NewStorageVolumeMounts() []corev1.VolumeMount { SubPath: "seaweed", }) - if r.TLS { - mounts = append(mounts, - corev1.VolumeMount{ - Name: "storage-tls-secret", - MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls", r.Name), - ReadOnly: true, - }) - } + // TODO + // if r.TLS { + // mounts = append(mounts, + // corev1.VolumeMount{ + // Name: "storage-tls-secret", + // MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-storage-tls", r.Name), + // ReadOnly: true, + // }) + // } return mounts } @@ -2231,14 +2237,15 @@ func (r *TestResources) NewDatabaseVolumeMounts() []corev1.VolumeMount { SubPath: "postgres", }) - if r.TLS { - mounts = append(mounts, - corev1.VolumeMount{ - Name: "database-tls-secret", - MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls", r.Name), - ReadOnly: true, - }) - } + // TODO + // if r.TLS { + // mounts = append(mounts, + // corev1.VolumeMount{ + // Name: "database-tls-secret", + // MountPath: fmt.Sprintf("/var/run/secrets/operator.cryostat.io/%s-database-tls", r.Name), + // ReadOnly: true, + // }) + // } return mounts } @@ -2385,9 +2392,9 @@ func (r *TestResources) NewDatasourceLivenessProbe() *corev1.Probe { func (r *TestResources) NewStorageLivenessProbe() *corev1.Probe { protocol := corev1.URISchemeHTTP - if r.TLS { - protocol = corev1.URISchemeHTTPS - } + // if r.TLS { + // protocol = corev1.URISchemeHTTPS + // } return &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ From 21d036c134eb671cb3568cdb6852c877aca2e44c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 5 Nov 2024 10:56:11 -0500 Subject: [PATCH 14/17] remove authproxy uid from bundle --- .../manifests/cryostat-operator.clusterserviceversion.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index b6029221..11010d3f 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -16,11 +16,6 @@ metadata: "reportOptions": { "replicas": 0 }, - "securityOptions": { - "authProxySecurityContext": { - "runAsUser": 1001 - } - }, "storageOptions": { "pvc": { "annotations": {}, From bcd0a3ec0694b3f305481b6e360cbbc8aca04ac8 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 5 Nov 2024 14:11:48 -0500 Subject: [PATCH 15/17] fix(tls): use fixed-length cert CommonNames --- .../cryostat-operator.clusterserviceversion.yaml | 2 +- .../common/resource_definitions/certificates.go | 11 ++++++----- internal/controllers/constants/constants.go | 6 ++++++ internal/test/resources.go | 10 +++++----- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 11010d3f..5e2f56e3 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -30,7 +30,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-10-10T18:16:26Z" + createdAt: "2024-11-05T19:02:47Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/internal/controllers/common/resource_definitions/certificates.go b/internal/controllers/common/resource_definitions/certificates.go index 2ea2a2ba..6356160f 100644 --- a/internal/controllers/common/resource_definitions/certificates.go +++ b/internal/controllers/common/resource_definitions/certificates.go @@ -20,6 +20,7 @@ import ( certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" certMeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/cryostatio/cryostat-operator/internal/controllers/common" + "github.com/cryostatio/cryostat-operator/internal/controllers/constants" "github.com/cryostatio/cryostat-operator/internal/controllers/model" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -62,7 +63,7 @@ func NewCryostatCACert(gvk *schema.GroupVersionKind, cr *model.CryostatInstance) Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("ca.%s.cert-manager", cr.Name), + CommonName: constants.CryostatCATLSCommonName, SecretName: common.ClusterUniqueNameWithPrefix(gvk, "ca", cr.Name, cr.InstallNamespace), IssuerRef: certMeta.ObjectReference{ Name: cr.Name + "-self-signed", @@ -79,7 +80,7 @@ func NewCryostatCert(cr *model.CryostatInstance, keystoreSecretName string) *cer Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.CryostatTLSCommonName, DNSNames: []string{ cr.Name, fmt.Sprintf("%s.%s.svc", cr.Name, cr.InstallNamespace), @@ -115,7 +116,7 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate { Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s-reports.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.ReportsTLSCommonName, DNSNames: []string{ cr.Name + "-reports", fmt.Sprintf("%s-reports.%s.svc", cr.Name, cr.InstallNamespace), @@ -189,7 +190,7 @@ func NewAgentCert(cr *model.CryostatInstance, namespace string, gvk *schema.Grou Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("*.%s.pod", namespace), + CommonName: constants.AgentsTLSCommonName, DNSNames: []string{ fmt.Sprintf("*.%s.pod", namespace), }, @@ -212,7 +213,7 @@ func NewAgentProxyCert(cr *model.CryostatInstance) *certv1.Certificate { Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s-agent.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.AgentAuthProxyTLSCommonName, DNSNames: []string{ cr.Name + "-agent", fmt.Sprintf("%s-agent.%s.svc", cr.Name, cr.InstallNamespace), diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index 59b7bd9b..5b3b26ec 100644 --- a/internal/controllers/constants/constants.go +++ b/internal/controllers/constants/constants.go @@ -50,4 +50,10 @@ const ( targetNamespaceCRLabelPrefix = "operator.cryostat.io/" TargetNamespaceCRNameLabel = targetNamespaceCRLabelPrefix + "name" TargetNamespaceCRNamespaceLabel = targetNamespaceCRLabelPrefix + "namespace" + + CryostatCATLSCommonName = "cryostat-ca-cert-manager" + CryostatTLSCommonName = "cryostat" + ReportsTLSCommonName = "cryostat-reports" + AgentsTLSCommonName = "cryostat-agent" + AgentAuthProxyTLSCommonName = "cryostat-agent-proxy" ) diff --git a/internal/test/resources.go b/internal/test/resources.go index dea41e0b..5bfc2bac 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1175,7 +1175,7 @@ func (r *TestResources) NewCryostatCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+".%s.svc", r.Namespace), + CommonName: "cryostat", DNSNames: []string{ r.Name, fmt.Sprintf(r.Name+".%s.svc", r.Namespace), @@ -1213,7 +1213,7 @@ func (r *TestResources) NewReportsCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+"-reports.%s.svc", r.Namespace), + CommonName: "cryostat-reports", DNSNames: []string{ r.Name + "-reports", fmt.Sprintf(r.Name+"-reports.%s.svc", r.Namespace), @@ -1265,7 +1265,7 @@ func (r *TestResources) NewAgentProxyCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+"-agent.%s.svc", r.Namespace), + CommonName: "cryostat-agent-proxy", DNSNames: []string{ r.Name + "-agent", fmt.Sprintf(r.Name+"-agent.%s.svc", r.Namespace), @@ -1317,7 +1317,7 @@ func (r *TestResources) NewCACert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("ca.%s.cert-manager", r.Name), + CommonName: "cryostat-ca-cert-manager", SecretName: r.getClusterUniqueNameForCA(), IssuerRef: certMeta.ObjectReference{ Name: r.Name + "-self-signed", @@ -1335,7 +1335,7 @@ func (r *TestResources) NewAgentCert(namespace string) *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("*.%s.pod", namespace), + CommonName: "cryostat-agent", DNSNames: []string{ fmt.Sprintf("*.%s.pod", namespace), }, From 0d7173cb8120fab9b5805aa56c12494ca772b94a Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 5 Nov 2024 14:56:17 -0500 Subject: [PATCH 16/17] fixup! Merge branch 'tls-common-name' into 814-separate-pods --- bundle/manifests/cryostat-operator.clusterserviceversion.yaml | 2 +- .../controllers/common/resource_definitions/certificates.go | 4 ++-- internal/controllers/constants/constants.go | 2 ++ internal/test/resources.go | 4 ++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 5e2f56e3..b942314c 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -30,7 +30,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-11-05T19:02:47Z" + createdAt: "2024-11-05T19:41:16Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/internal/controllers/common/resource_definitions/certificates.go b/internal/controllers/common/resource_definitions/certificates.go index 6356160f..2598d8c4 100644 --- a/internal/controllers/common/resource_definitions/certificates.go +++ b/internal/controllers/common/resource_definitions/certificates.go @@ -140,7 +140,7 @@ func NewDatabaseCert(cr *model.CryostatInstance) *certv1.Certificate { Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s-database.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.DatabaseTLSCommonName, DNSNames: []string{ cr.Name + "-database", fmt.Sprintf("%s-database.%s.svc", cr.Name, cr.InstallNamespace), @@ -164,7 +164,7 @@ func NewStorageCert(cr *model.CryostatInstance) *certv1.Certificate { Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s-storage.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.StorageTLSCommonName, DNSNames: []string{ cr.Name + "-storage", fmt.Sprintf("%s-storage.%s.svc", cr.Name, cr.InstallNamespace), diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index 5b3b26ec..de70581f 100644 --- a/internal/controllers/constants/constants.go +++ b/internal/controllers/constants/constants.go @@ -53,6 +53,8 @@ const ( CryostatCATLSCommonName = "cryostat-ca-cert-manager" CryostatTLSCommonName = "cryostat" + DatabaseTLSCommonName = "cryostat-db" + StorageTLSCommonName = "cryostat-storage" ReportsTLSCommonName = "cryostat-reports" AgentsTLSCommonName = "cryostat-agent" AgentAuthProxyTLSCommonName = "cryostat-agent-proxy" diff --git a/internal/test/resources.go b/internal/test/resources.go index 5bfc2bac..71c6f929 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1239,7 +1239,7 @@ func (r *TestResources) NewDatabaseCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+"-database.%s.svc", r.Namespace), + CommonName: "cryostat-database", DNSNames: []string{ r.Name + "-database", fmt.Sprintf(r.Name+"-database.%s.svc", r.Namespace), @@ -1291,7 +1291,7 @@ func (r *TestResources) NewStorageCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+"-storage.%s.svc", r.Namespace), + CommonName: "cryostat-storage", DNSNames: []string{ r.Name + "-storage", fmt.Sprintf(r.Name+"-storage.%s.svc", r.Namespace), From 4ce5ccaada6b51fb8418fa26a35294c4eb9eac74 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 20 Dec 2024 11:10:09 -0500 Subject: [PATCH 17/17] fix(tls): use fixed-length cert CommonNames (#968) * fix(tls): use fixed-length cert CommonNames * certificate change recreation * delete cert secret so they are also recreated --- ...yostat-operator.clusterserviceversion.yaml | 2 +- internal/controllers/certmanager.go | 25 +++++++- .../resource_definitions/certificates.go | 11 ++-- internal/controllers/constants/constants.go | 6 ++ internal/controllers/reconciler_test.go | 60 +++++++++++++++++++ internal/test/resources.go | 35 +++++++++-- 6 files changed, 125 insertions(+), 14 deletions(-) diff --git a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml index 9aa26834..297b8298 100644 --- a/bundle/manifests/cryostat-operator.clusterserviceversion.yaml +++ b/bundle/manifests/cryostat-operator.clusterserviceversion.yaml @@ -30,7 +30,7 @@ metadata: capabilities: Seamless Upgrades categories: Monitoring, Developer Tools containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev - createdAt: "2024-10-10T18:16:26Z" + createdAt: "2024-11-05T19:02:47Z" description: JVM monitoring and profiling tool operatorframework.io/initialization-resource: |- { diff --git a/internal/controllers/certmanager.go b/internal/controllers/certmanager.go index 35df44c1..9305c77e 100644 --- a/internal/controllers/certmanager.go +++ b/internal/controllers/certmanager.go @@ -24,6 +24,7 @@ import ( "github.com/cryostatio/cryostat-operator/internal/controllers/common" resources "github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions" "github.com/cryostatio/cryostat-operator/internal/controllers/model" + "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -398,25 +399,43 @@ func (r *Reconciler) reconcileAgentCertificate(ctx context.Context, cert *certv1 return nil } +var errCertificateModified error = errors.New("certificate has been modified") + func (r *Reconciler) createOrUpdateCertificate(ctx context.Context, cert *certv1.Certificate, owner metav1.Object) error { - certSpec := cert.Spec.DeepCopy() + certCopy := cert.DeepCopy() op, err := controllerutil.CreateOrUpdate(ctx, r.Client, cert, func() error { if owner != nil { if err := controllerutil.SetControllerReference(owner, cert, r.Scheme); err != nil { return err } } - // Update Certificate spec - cert.Spec = *certSpec + + if cert.CreationTimestamp.IsZero() { + cert.Spec = certCopy.Spec + } else if !cmp.Equal(cert.Spec, certCopy.Spec) { + return errCertificateModified + } + return nil }) if err != nil { + if err == errCertificateModified { + return r.recreateCertificate(ctx, certCopy, owner) + } return err } r.Log.Info(fmt.Sprintf("Certificate %s", op), "name", cert.Name, "namespace", cert.Namespace) return nil } +func (r *Reconciler) recreateCertificate(ctx context.Context, cert *certv1.Certificate, owner metav1.Object) error { + err := r.deleteCertWithSecret(ctx, cert) + if err != nil { + return err + } + return r.createOrUpdateCertificate(ctx, cert, owner) +} + func newKeystoreSecret(cr *model.CryostatInstance) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controllers/common/resource_definitions/certificates.go b/internal/controllers/common/resource_definitions/certificates.go index be757c74..55bf7645 100644 --- a/internal/controllers/common/resource_definitions/certificates.go +++ b/internal/controllers/common/resource_definitions/certificates.go @@ -20,6 +20,7 @@ import ( certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" certMeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/cryostatio/cryostat-operator/internal/controllers/common" + "github.com/cryostatio/cryostat-operator/internal/controllers/constants" "github.com/cryostatio/cryostat-operator/internal/controllers/model" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -62,7 +63,7 @@ func NewCryostatCACert(gvk *schema.GroupVersionKind, cr *model.CryostatInstance) Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("ca.%s.cert-manager", cr.Name), + CommonName: constants.CryostatCATLSCommonName, SecretName: common.ClusterUniqueNameWithPrefix(gvk, "ca", cr.Name, cr.InstallNamespace), IssuerRef: certMeta.ObjectReference{ Name: cr.Name + "-self-signed", @@ -79,7 +80,7 @@ func NewCryostatCert(cr *model.CryostatInstance, keystoreSecretName string) *cer Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.CryostatTLSCommonName, DNSNames: []string{ cr.Name, fmt.Sprintf("%s.%s.svc", cr.Name, cr.InstallNamespace), @@ -115,7 +116,7 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate { Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s-reports.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.ReportsTLSCommonName, DNSNames: []string{ cr.Name + "-reports", fmt.Sprintf("%s-reports.%s.svc", cr.Name, cr.InstallNamespace), @@ -140,7 +141,7 @@ func NewAgentCert(cr *model.CryostatInstance, namespace string, gvk *schema.Grou Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("*.%s.pod", namespace), + CommonName: constants.AgentsTLSCommonName, DNSNames: []string{ fmt.Sprintf("*.%s.pod", namespace), }, @@ -163,7 +164,7 @@ func NewAgentProxyCert(cr *model.CryostatInstance) *certv1.Certificate { Namespace: cr.InstallNamespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("%s-agent.%s.svc", cr.Name, cr.InstallNamespace), + CommonName: constants.AgentAuthProxyTLSCommonName, DNSNames: []string{ cr.Name + "-agent", fmt.Sprintf("%s-agent.%s.svc", cr.Name, cr.InstallNamespace), diff --git a/internal/controllers/constants/constants.go b/internal/controllers/constants/constants.go index 59b7bd9b..5b3b26ec 100644 --- a/internal/controllers/constants/constants.go +++ b/internal/controllers/constants/constants.go @@ -50,4 +50,10 @@ const ( targetNamespaceCRLabelPrefix = "operator.cryostat.io/" TargetNamespaceCRNameLabel = targetNamespaceCRLabelPrefix + "name" TargetNamespaceCRNamespaceLabel = targetNamespaceCRLabelPrefix + "namespace" + + CryostatCATLSCommonName = "cryostat-ca-cert-manager" + CryostatTLSCommonName = "cryostat" + ReportsTLSCommonName = "cryostat-reports" + AgentsTLSCommonName = "cryostat-agent" + AgentAuthProxyTLSCommonName = "cryostat-agent-proxy" ) diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index 612c69f4..40a71f33 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -1795,6 +1795,66 @@ func (c *controllerTest) commonTests() { t.expectCertificates() }) }) + Context("with modified certificates", func() { + var oldCerts []*certv1.Certificate + BeforeEach(func() { + t.objs = append(t.objs, t.NewCryostat().Object, t.OtherCAIssuer()) + oldCerts = []*certv1.Certificate{ + t.OtherCACert(), + t.OtherAgentProxyCert(), + t.OtherCryostatCert(), + t.OtherReportsCert(), + } + // Add an annotation for each cert, the test will assert that + // the annotation is gone. + for i, cert := range oldCerts { + metav1.SetMetaDataAnnotation(&oldCerts[i].ObjectMeta, "bad", "cert") + t.objs = append(t.objs, cert) + } + }) + JustBeforeEach(func() { + cr := t.getCryostatInstance() + for _, cert := range oldCerts { + // Make the old certs owned by the Cryostat CR + err := controllerutil.SetControllerReference(cr.Object, cert, t.Client.Scheme()) + Expect(err).ToNot(HaveOccurred()) + err = t.Client.Update(context.Background(), cert) + Expect(err).ToNot(HaveOccurred()) + } + t.reconcileCryostatFully() + }) + It("should recreate certificates", func() { + t.expectCertificates() + }) + }) + Context("with a modified certificate TLS CommonName", func() { + var oldCerts []*certv1.Certificate + BeforeEach(func() { + oldCerts = []*certv1.Certificate{ + t.NewCryostatCert(), + t.NewReportsCert(), + t.NewAgentProxyCert(), + } + t.objs = append(t.objs, t.NewCryostat().Object, t.OtherCAIssuer()) + for _, cert := range oldCerts { + t.objs = append(t.objs, cert) + } + }) + JustBeforeEach(func() { + cr := t.getCryostatInstance() + for _, cert := range oldCerts { + // Make the old certs owned by the Cryostat CR + err := controllerutil.SetControllerReference(cr.Object, cert, t.Client.Scheme()) + Expect(err).ToNot(HaveOccurred()) + err = t.Client.Update(context.Background(), cert) + Expect(err).ToNot(HaveOccurred()) + } + t.reconcileCryostatFully() + }) + It("should recreate certificates", func() { + t.expectCertificates() + }) + }) Context("reconciling a multi-namespace request", func() { targetNamespaces := []string{"multi-test-one", "multi-test-two"} diff --git a/internal/test/resources.go b/internal/test/resources.go index bc74bc9f..6c1adf3d 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -1053,7 +1053,7 @@ func (r *TestResources) NewCryostatCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+".%s.svc", r.Namespace), + CommonName: "cryostat", DNSNames: []string{ r.Name, fmt.Sprintf(r.Name+".%s.svc", r.Namespace), @@ -1084,6 +1084,12 @@ func (r *TestResources) NewCryostatCert() *certv1.Certificate { } } +func (r *TestResources) OtherCryostatCert() *certv1.Certificate { + cert := r.NewCryostatCert() + cert.Spec.CommonName = fmt.Sprintf("%s.%s.svc", r.Name, r.Namespace) + return cert +} + func (r *TestResources) NewReportsCert() *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1091,7 +1097,7 @@ func (r *TestResources) NewReportsCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+"-reports.%s.svc", r.Namespace), + CommonName: "cryostat-reports", DNSNames: []string{ r.Name + "-reports", fmt.Sprintf(r.Name+"-reports.%s.svc", r.Namespace), @@ -1110,6 +1116,12 @@ func (r *TestResources) NewReportsCert() *certv1.Certificate { } } +func (r *TestResources) OtherReportsCert() *certv1.Certificate { + cert := r.NewReportsCert() + cert.Spec.CommonName = fmt.Sprintf("%s-reports.%s.svc", r.Name, r.Namespace) + return cert +} + func (r *TestResources) NewAgentProxyCert() *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1117,7 +1129,7 @@ func (r *TestResources) NewAgentProxyCert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf(r.Name+"-agent.%s.svc", r.Namespace), + CommonName: "cryostat-agent-proxy", DNSNames: []string{ r.Name + "-agent", fmt.Sprintf(r.Name+"-agent.%s.svc", r.Namespace), @@ -1136,6 +1148,12 @@ func (r *TestResources) NewAgentProxyCert() *certv1.Certificate { } } +func (r *TestResources) OtherAgentProxyCert() *certv1.Certificate { + cert := r.NewAgentProxyCert() + cert.Spec.CommonName = fmt.Sprintf("%s-agent.%s.svc", r.Name, r.Namespace) + return cert +} + func (r *TestResources) NewCACert() *certv1.Certificate { return &certv1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1143,7 +1161,7 @@ func (r *TestResources) NewCACert() *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("ca.%s.cert-manager", r.Name), + CommonName: "cryostat-ca-cert-manager", SecretName: r.getClusterUniqueNameForCA(), IssuerRef: certMeta.ObjectReference{ Name: r.Name + "-self-signed", @@ -1153,6 +1171,13 @@ func (r *TestResources) NewCACert() *certv1.Certificate { } } +func (r *TestResources) OtherCACert() *certv1.Certificate { + cert := r.NewCACert() + cert.Spec.CommonName = fmt.Sprintf("ca.%s.cert-manager", r.Name) + cert.Spec.SecretName = r.Name + "-ca" + return cert +} + func (r *TestResources) NewAgentCert(namespace string) *certv1.Certificate { name := r.getClusterUniqueNameForAgent(namespace) return &certv1.Certificate{ @@ -1161,7 +1186,7 @@ func (r *TestResources) NewAgentCert(namespace string) *certv1.Certificate { Namespace: r.Namespace, }, Spec: certv1.CertificateSpec{ - CommonName: fmt.Sprintf("*.%s.pod", namespace), + CommonName: "cryostat-agent", DNSNames: []string{ fmt.Sprintf("*.%s.pod", namespace), },