From 98d78fea5714ba4f2efecd2c7b56676d17922e78 Mon Sep 17 00:00:00 2001 From: Vu Dinh Date: Thu, 30 Nov 2023 02:23:03 -0500 Subject: [PATCH] [carry] Add ownership annotations to new and existing olm-managed secrets As a part of certificates ownership work, all of new and existing secrets that are created with certificate information need to have ownership annotations. New secrets that are created with OLM certresources controller will have new ownership annotations injected at creation/update time. The existing olm-managed secrets that don't have the required annotations will get reconciled at startup when olm operator is restarted/redeployed. This PR only add OpenShift owning component annotation and the description annotation can be added later. Signed-off-by: Vu Dinh --- manifests/0000_50_olm_00-pprof-secret.yaml | 1 + .../0000_50_olm_00-pprof-secret.yaml | 1 + scripts/generate_crds_manifests.sh | 9 +++---- .../cmd/olm/main.go | 5 ++++ .../pkg/controller/install/certresources.go | 13 ++++++++-- .../controller/install/certresources_test.go | 6 ++--- .../pkg/controller/operators/olm/operator.go | 24 +++++++++++++++++++ .../cmd/olm/main.go | 5 ++++ .../pkg/controller/install/certresources.go | 13 ++++++++-- .../pkg/controller/operators/olm/operator.go | 24 +++++++++++++++++++ 10 files changed, 90 insertions(+), 11 deletions(-) diff --git a/manifests/0000_50_olm_00-pprof-secret.yaml b/manifests/0000_50_olm_00-pprof-secret.yaml index d77a5c076c..e438c13409 100644 --- a/manifests/0000_50_olm_00-pprof-secret.yaml +++ b/manifests/0000_50_olm_00-pprof-secret.yaml @@ -5,6 +5,7 @@ metadata: include.release.openshift.io/ibm-cloud-managed: "true" include.release.openshift.io/self-managed-high-availability: "true" release.openshift.io/create-only: "true" + openshift.io/owning-component: "Operator Framework / operator-lifecycle-manager" capability.openshift.io/name: "OperatorLifecycleManager" name: pprof-cert namespace: openshift-operator-lifecycle-manager diff --git a/microshift-manifests/0000_50_olm_00-pprof-secret.yaml b/microshift-manifests/0000_50_olm_00-pprof-secret.yaml index d77a5c076c..e438c13409 100644 --- a/microshift-manifests/0000_50_olm_00-pprof-secret.yaml +++ b/microshift-manifests/0000_50_olm_00-pprof-secret.yaml @@ -5,6 +5,7 @@ metadata: include.release.openshift.io/ibm-cloud-managed: "true" include.release.openshift.io/self-managed-high-availability: "true" release.openshift.io/create-only: "true" + openshift.io/owning-component: "Operator Framework / operator-lifecycle-manager" capability.openshift.io/name: "OperatorLifecycleManager" name: pprof-cert namespace: openshift-operator-lifecycle-manager diff --git a/scripts/generate_crds_manifests.sh b/scripts/generate_crds_manifests.sh index 6acf0255af..07cfe900c0 100755 --- a/scripts/generate_crds_manifests.sh +++ b/scripts/generate_crds_manifests.sh @@ -346,6 +346,7 @@ metadata: include.release.openshift.io/ibm-cloud-managed: "true" include.release.openshift.io/self-managed-high-availability: "true" release.openshift.io/create-only: "true" + openshift.io/owning-component: "Operator Framework / operator-lifecycle-manager" name: pprof-cert namespace: openshift-operator-lifecycle-manager type: kubernetes.io/tls @@ -488,7 +489,7 @@ add_ibm_managed_cloud_annotations "${ROOT_DIR}/manifests" find "${ROOT_DIR}/manifests" -type f -exec $SED -i "/^#/d" {} \; find "${ROOT_DIR}/manifests" -type f -exec $SED -i "1{/---/d}" {} \; -# (anik120): uncomment this once https://issues.redhat.com/browse/OLM-2695 is Done. +# (anik120): uncomment this once https://issues.redhat.com/browse/OLM-2695 is Done. #${YQ} delete --inplace -d'1' manifests/0000_50_olm_00-namespace.yaml 'metadata.labels."pod-security.kubernetes.io/enforce*"' # Unlike the namespaces shipped in the upstream version, the openshift-operator-lifecycle-manager and openshift-operator @@ -505,7 +506,7 @@ cp "${ROOT_DIR}"/manifests/* "${ROOT_DIR}/microshift-manifests/" # There are some differences that we need to take care of: # - The manifests require a kustomization.yaml file # - We don't need the specific ibm-cloud-managed manifests -# - We need to adapt some of the manifests to be compatible with microshift as there's no +# - We need to adapt some of the manifests to be compatible with microshift as there's no # ClusterVersion or ClusterOperator in microshift # Create the kustomization file @@ -530,7 +531,7 @@ for file in ${microshift_manifests_files}; do fi done echo " - $(realpath --relative-to "${ROOT_DIR}/microshift-manifests" "${file}")" >> "${ROOT_DIR}/microshift-manifests/kustomization.yaml" -done +done # Now we need to get rid of these args from the olm-operator deployment: # @@ -539,7 +540,7 @@ done # - --writePackageServerStatusName # - operator-lifecycle-manager-packageserver # -${SED} -i '/- --writeStatusName/,+3d' ${ROOT_DIR}/microshift-manifests/0000_50_olm_07-olm-operator.deployment.yaml +${SED} -i '/- --writeStatusName/,+3d' ${ROOT_DIR}/microshift-manifests/0000_50_olm_07-olm-operator.deployment.yaml # Replace the namespace openshift, as it doesn't exist on microshift, in the rbac file ${SED} -i 's/ namespace: openshift/ namespace: openshift-operator-lifecycle-manager/g' ${ROOT_DIR}/microshift-manifests/0000_50_olm_15-csv-viewer.rbac.yaml diff --git a/staging/operator-lifecycle-manager/cmd/olm/main.go b/staging/operator-lifecycle-manager/cmd/olm/main.go index 6d76606dc3..58570fa590 100644 --- a/staging/operator-lifecycle-manager/cmd/olm/main.go +++ b/staging/operator-lifecycle-manager/cmd/olm/main.go @@ -224,6 +224,11 @@ func main() { go monitor.Run(op.Done()) } + // Reconcile all olm-managed secrets to add ownership annotations if not existed + if err = op.EnsureSecretOwnershipAnnotations(); err != nil { + logger.WithError(err).Fatal("error injecting ownership annotations to existing olm-managed secrets") + } + // Start the controller manager if err := mgr.Start(ctx); err != nil { logger.WithError(err).Fatal("controller manager stopped") diff --git a/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go b/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go index 45bc78d0c9..98290383ef 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go +++ b/staging/operator-lifecycle-manager/pkg/controller/install/certresources.go @@ -42,6 +42,10 @@ const ( // olm managed label OLMManagedLabelKey = "olm.managed" OLMManagedLabelValue = "true" + // Use this const for now to avoid openshift/api bump + // TODO: Bump openshift/api and remove this const + OpenShiftComponent = "openshift.io/owning-component" + OLMOwnershipAnnotation = "Operator Framework / operator-lifecycle-manager" ) type certResource interface { @@ -300,6 +304,11 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo } caHash := certs.PEMSHA256(caPEM) + annotations := map[string]string{ + OpenShiftComponent: OLMOwnershipAnnotation, + OLMCAHashAnnotationKey: caHash, + } + secret := &corev1.Secret{ Data: map[string][]byte{ "tls.crt": certPEM, @@ -310,8 +319,8 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo } secret.SetName(SecretName(service.GetName())) secret.SetNamespace(i.owner.GetNamespace()) - secret.SetAnnotations(map[string]string{OLMCAHashAnnotationKey: caHash}) secret.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}) + secret.SetAnnotations(annotations) existingSecret, err := i.strategyClient.GetOpLister().CoreV1().SecretLister().Secrets(i.owner.GetNamespace()).Get(secret.GetName()) if err == nil { @@ -322,7 +331,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo // Attempt an update // TODO: Check that the secret was not modified - if existingCAPEM, ok := existingSecret.Data[OLMCAPEMKey]; ok && !ShouldRotateCerts(i.owner.(*v1alpha1.ClusterServiceVersion)) { + if existingCAPEM, ok := existingSecret.Data[OLMCAPEMKey]; ok && !ShouldRotateCerts(i.owner.(*v1alpha1.ClusterServiceVersion)) && existingSecret.Annotations[OpenShiftComponent] != "" { logger.Warnf("reusing existing cert %s", secret.GetName()) secret = existingSecret caPEM = existingCAPEM diff --git a/staging/operator-lifecycle-manager/pkg/controller/install/certresources_test.go b/staging/operator-lifecycle-manager/pkg/controller/install/certresources_test.go index ad8e1041b4..39b42b93d9 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/install/certresources_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/install/certresources_test.go @@ -184,7 +184,7 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "test-service-cert", Namespace: namespace, - Annotations: map[string]string{OLMCAHashAnnotationKey: caHash}, + Annotations: map[string]string{OLMCAHashAnnotationKey: caHash, OpenShiftComponent: OLMOwnershipAnnotation}, Labels: map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}, }, Data: map[string][]byte{ @@ -413,7 +413,7 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "test-service-cert", Namespace: namespace, - Annotations: map[string]string{OLMCAHashAnnotationKey: caHash}, + Annotations: map[string]string{OLMCAHashAnnotationKey: caHash, OpenShiftComponent: OLMOwnershipAnnotation}, Labels: map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}, }, Data: map[string][]byte{ @@ -635,7 +635,7 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "test-service-cert", Namespace: namespace, - Annotations: map[string]string{OLMCAHashAnnotationKey: caHash}, + Annotations: map[string]string{OLMCAHashAnnotationKey: caHash, OpenShiftComponent: OLMOwnershipAnnotation}, Labels: map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}, OwnerReferences: []metav1.OwnerReference{ ownerutil.NonBlockingOwner(&v1alpha1.ClusterServiceVersion{}), diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go index 5e3c294bd1..e49a1e0876 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go @@ -2900,3 +2900,27 @@ func (a *Operator) ensureLabels(in *v1alpha1.ClusterServiceVersion, labelSets .. out, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(out.GetNamespace()).Update(context.TODO(), out, metav1.UpdateOptions{}) return out, err } + +// syncSecret adds required ownership annotations to olm-managed secrets +func (a *Operator) EnsureSecretOwnershipAnnotations() error { + secrets, err := a.lister.CoreV1().SecretLister().List(labels.SelectorFromSet(labels.Set{install.OLMManagedLabelKey: install.OLMManagedLabelValue})) + if err != nil { + return err + } + for _, secret := range secrets { + if secret.Annotations[install.OpenShiftComponent] == "" { + secret.Annotations[install.OpenShiftComponent] = install.OLMOwnershipAnnotation + logger := a.logger.WithFields(logrus.Fields{ + "name": secret.GetName(), + "namespace": secret.GetNamespace(), + "self": secret.GetSelfLink(), + }) + logger.Debug("injecting ownership annotations to existing secret") + if _, updateErr := a.opClient.UpdateSecret(secret); updateErr != nil { + logger.WithError(err).Warn("error adding ownership annotations to existing secret") + return err + } + } + } + return nil +} diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go index 6d76606dc3..58570fa590 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go @@ -224,6 +224,11 @@ func main() { go monitor.Run(op.Done()) } + // Reconcile all olm-managed secrets to add ownership annotations if not existed + if err = op.EnsureSecretOwnershipAnnotations(); err != nil { + logger.WithError(err).Fatal("error injecting ownership annotations to existing olm-managed secrets") + } + // Start the controller manager if err := mgr.Start(ctx); err != nil { logger.WithError(err).Fatal("controller manager stopped") diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/certresources.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/certresources.go index 45bc78d0c9..98290383ef 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/certresources.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install/certresources.go @@ -42,6 +42,10 @@ const ( // olm managed label OLMManagedLabelKey = "olm.managed" OLMManagedLabelValue = "true" + // Use this const for now to avoid openshift/api bump + // TODO: Bump openshift/api and remove this const + OpenShiftComponent = "openshift.io/owning-component" + OLMOwnershipAnnotation = "Operator Framework / operator-lifecycle-manager" ) type certResource interface { @@ -300,6 +304,11 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo } caHash := certs.PEMSHA256(caPEM) + annotations := map[string]string{ + OpenShiftComponent: OLMOwnershipAnnotation, + OLMCAHashAnnotationKey: caHash, + } + secret := &corev1.Secret{ Data: map[string][]byte{ "tls.crt": certPEM, @@ -310,8 +319,8 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo } secret.SetName(SecretName(service.GetName())) secret.SetNamespace(i.owner.GetNamespace()) - secret.SetAnnotations(map[string]string{OLMCAHashAnnotationKey: caHash}) secret.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue}) + secret.SetAnnotations(annotations) existingSecret, err := i.strategyClient.GetOpLister().CoreV1().SecretLister().Secrets(i.owner.GetNamespace()).Get(secret.GetName()) if err == nil { @@ -322,7 +331,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo // Attempt an update // TODO: Check that the secret was not modified - if existingCAPEM, ok := existingSecret.Data[OLMCAPEMKey]; ok && !ShouldRotateCerts(i.owner.(*v1alpha1.ClusterServiceVersion)) { + if existingCAPEM, ok := existingSecret.Data[OLMCAPEMKey]; ok && !ShouldRotateCerts(i.owner.(*v1alpha1.ClusterServiceVersion)) && existingSecret.Annotations[OpenShiftComponent] != "" { logger.Warnf("reusing existing cert %s", secret.GetName()) secret = existingSecret caPEM = existingCAPEM diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go index 5e3c294bd1..e49a1e0876 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go @@ -2900,3 +2900,27 @@ func (a *Operator) ensureLabels(in *v1alpha1.ClusterServiceVersion, labelSets .. out, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(out.GetNamespace()).Update(context.TODO(), out, metav1.UpdateOptions{}) return out, err } + +// syncSecret adds required ownership annotations to olm-managed secrets +func (a *Operator) EnsureSecretOwnershipAnnotations() error { + secrets, err := a.lister.CoreV1().SecretLister().List(labels.SelectorFromSet(labels.Set{install.OLMManagedLabelKey: install.OLMManagedLabelValue})) + if err != nil { + return err + } + for _, secret := range secrets { + if secret.Annotations[install.OpenShiftComponent] == "" { + secret.Annotations[install.OpenShiftComponent] = install.OLMOwnershipAnnotation + logger := a.logger.WithFields(logrus.Fields{ + "name": secret.GetName(), + "namespace": secret.GetNamespace(), + "self": secret.GetSelfLink(), + }) + logger.Debug("injecting ownership annotations to existing secret") + if _, updateErr := a.opClient.UpdateSecret(secret); updateErr != nil { + logger.WithError(err).Warn("error adding ownership annotations to existing secret") + return err + } + } + } + return nil +}