Skip to content

Commit

Permalink
[carry] Add ownership annotations to new and existing olm-managed sec…
Browse files Browse the repository at this point in the history
…rets

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 <[email protected]>
  • Loading branch information
dinhxuanvu committed Nov 30, 2023
1 parent cc54174 commit 98d78fe
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 11 deletions.
1 change: 1 addition & 0 deletions manifests/0000_50_olm_00-pprof-secret.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions microshift-manifests/0000_50_olm_00-pprof-secret.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions scripts/generate_crds_manifests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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:
#
Expand All @@ -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
5 changes: 5 additions & 0 deletions staging/operator-lifecycle-manager/cmd/olm/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 98d78fe

Please sign in to comment.