Skip to content

Commit

Permalink
Add description annotations to service CA secret and configmap
Browse files Browse the repository at this point in the history
Service CA generates a secret and configmap for its signing key.
These secret and configmap need to have description annotation
to conform to the new cert ownership validation.

For all secrets that are created by service CA for services, an
ownership and description annotations are generated to indicate
service CA owns these secrets and its corresponding service owner.

Signed-off-by: Vu Dinh <vudinh@outlook.com>
  • Loading branch information
dinhxuanvu committed Nov 30, 2023
1 parent fe88761 commit 5c0ebfe
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 37 deletions.
1 change: 1 addition & 0 deletions bindata/v4.0.0/controller/signing-cabundle.yaml
Expand Up @@ -5,5 +5,6 @@ metadata:
name: signing-cabundle
annotations:
openshift.io/owning-component: service-ca
openshift.io/description: "Service CA configmap contains the data for the PEM-encoded CA signing bundle which will be injected to resources annotated with 'service.beta.openshift.io/inject-cabundle=true'"
data:
ca-bundle.crt:
1 change: 1 addition & 0 deletions bindata/v4.0.0/controller/signing-secret.yaml
Expand Up @@ -5,6 +5,7 @@ metadata:
name: signing-key
annotations:
openshift.io/owning-component: service-ca
openshift.io/description: "Service CA secret contains a signing key that will be used to issue a signed serving certificate/key pair to services annotated with 'service.beta.openshift.io/serving-cert-secret-name'"
type: kubernetes.io/tls
data:
tls.crt:
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/cabundleinjector/configmap.go
Expand Up @@ -2,6 +2,7 @@ package cabundleinjector

import (
"context"
"fmt"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -113,6 +114,7 @@ func (bi *configMapCABundleInjector) Sync(ctx context.Context, syncCtx factory.S
// set the owning-component unless someone else has claimed it.
if len(configMapCopy.Annotations[apiannotations.OpenShiftComponent]) == 0 {
configMapCopy.Annotations[apiannotations.OpenShiftComponent] = api.OwningJiraComponent
configMapCopy.Annotations[apiannotations.OpenShiftDescription] = fmt.Sprintf("Configmap is added/updated with a data item containing the CA signing bundle that can be used to verify service-serving certificates")
}

_, err = bi.client.ConfigMaps(configMapCopy.Namespace).Update(ctx, configMapCopy, metav1.UpdateOptions{})
Expand Down
Expand Up @@ -141,6 +141,7 @@ func (sc *serviceServingCertController) generateCert(ctx context.Context, servic
if err := toRequiredSecret(sc.dnsSuffix, sc.ca, sc.intermediateCACert, serviceCopy, secret); err != nil {
return err
}
setSecretOwnerDescription(secret, serviceCopy)

_, err := sc.secretClient.Secrets(serviceCopy.Namespace).Create(ctx, secret, metav1.CreateOptions{})
if err != nil && !kapierrors.IsAlreadyExists(err) {
Expand Down Expand Up @@ -328,9 +329,8 @@ func toBaseSecret(service *corev1.Service) *corev1.Secret {
Name: service.Annotations[api.ServingCertSecretAnnotation],
Namespace: service.Namespace,
Annotations: map[string]string{
api.ServiceUIDAnnotation: string(service.UID),
api.ServiceNameAnnotation: service.Name,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.ServiceUIDAnnotation: string(service.UID),
api.ServiceNameAnnotation: service.Name,
},
},
Type: corev1.SecretTypeTLS,
Expand All @@ -342,9 +342,8 @@ func toBaseSecret(service *corev1.Service) *corev1.Secret {
Name: service.Annotations[api.AlphaServingCertSecretAnnotation],
Namespace: service.Namespace,
Annotations: map[string]string{
api.AlphaServiceUIDAnnotation: string(service.UID),
api.AlphaServiceNameAnnotation: service.Name,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.AlphaServiceUIDAnnotation: string(service.UID),
api.AlphaServiceNameAnnotation: service.Name,
},
},
Type: corev1.SecretTypeTLS,
Expand Down Expand Up @@ -410,7 +409,6 @@ func toRequiredSecret(dnsSuffix string, ca *crypto.CA, intermediateCACert *x509.

secretCopy.Annotations[api.AlphaServingCertExpiryAnnotation] = servingCert.Certs[0].NotAfter.Format(time.RFC3339)
secretCopy.Annotations[api.ServingCertExpiryAnnotation] = servingCert.Certs[0].NotAfter.Format(time.RFC3339)
secretCopy.Annotations[apiannotations.OpenShiftComponent] = api.OwningJiraComponent

controller.EnsureOwnerRef(secretCopy, ownerRef(service))

Expand All @@ -433,3 +431,24 @@ func uidsEqual(secret *corev1.Secret, service *corev1.Service) bool {
return secret.Annotations[api.AlphaServiceUIDAnnotation] == suid ||
secret.Annotations[api.ServiceUIDAnnotation] == suid
}

// Set ownership and description annotations. Return true if there are changes on
// the two annotations
func setSecretOwnerDescription(secret *corev1.Secret, service *corev1.Service) bool {
changed := false
if secret.Annotations == nil {
secret.Annotations = map[string]string{}
changed = true
}
// Set service CA owner annotation to generated secret
if len(secret.Annotations[apiannotations.OpenShiftComponent]) == 0 {
secret.Annotations[apiannotations.OpenShiftComponent] = api.OwningJiraComponent
changed = true
}
// Generate a description for generated secret if not existed already
if len(secret.Annotations[apiannotations.OpenShiftDescription]) == 0 {
secret.Annotations[apiannotations.OpenShiftDescription] = fmt.Sprintf("Secret contains a pair signed serving certificate/key that is generated by Service CA operator for service/%s with hostname %s.%s.svc and is annotated to the service with annotating a service resource with 'service.beta.openshift.io/serving-cert-secret-name: %s'. The certificate is valid for 2 years.", service.Name, service.Name, service.Namespace, secret.Name)
changed = true
}
return changed
}
Expand Up @@ -37,6 +37,7 @@ const (
testServiceName = "svc-name"
testNamespace = "svc-namespace"
testSecretName = "new-secret"
descAnnotation = "Secret contains a pair signed serving certificate/key that is generated by Service CA operator for service svc-name/svc-namespace and is annotated to the service with annotating a service resource with 'service.beta.openshift.io/serving-cert-secret-name: new-secret'. The certificate is valid for 2 years."
testCertUnknownIssuer = `
-----BEGIN CERTIFICATE-----
MIIDETCCAfmgAwIBAgIUdbBKh0jOJxli4wl34q0TYJu8+n0wDQYJKoZIhvcNAQEL
Expand Down Expand Up @@ -146,9 +147,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
api.ServingCertCreatedByAnnotation: signerName,
},
expectedSecretAnnotations: map[string]string{
api.AlphaServiceUIDAnnotation: testServiceUID,
api.AlphaServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.AlphaServiceUIDAnnotation: testServiceUID,
api.AlphaServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
updateSecret: true,
updateService: true,
Expand All @@ -164,9 +166,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
api.ServingCertCreatedByAnnotation: signerName,
},
expectedSecretAnnotations: map[string]string{
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
updateSecret: true,
updateService: true,
Expand All @@ -183,9 +186,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
api.ServingCertCreatedByAnnotation: signerName,
},
expectedSecretAnnotations: map[string]string{
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
updateSecret: true,
updateService: true,
Expand All @@ -205,9 +209,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
api.ServingCertCreatedByAnnotation: signerName,
},
expectedSecretAnnotations: map[string]string{
api.AlphaServiceUIDAnnotation: testServiceUID,
api.AlphaServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.AlphaServiceUIDAnnotation: testServiceUID,
api.AlphaServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
updateSecret: true,
updateService: true,
Expand All @@ -227,9 +232,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
api.ServingCertCreatedByAnnotation: signerName,
},
expectedSecretAnnotations: map[string]string{
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
updateSecret: true,
updateService: true,
Expand Down Expand Up @@ -318,9 +324,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
},
secretData: generateServerCertPemForCA(t, ca, false),
expectedSecretAnnotations: map[string]string{
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
expectedServiceAnnotations: map[string]string{
api.ServingCertSecretAnnotation: testSecretName,
Expand All @@ -338,9 +345,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
},
secretData: generateServerCertPemForCA(t, ca, true),
expectedSecretAnnotations: map[string]string{
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
expectedServiceAnnotations: map[string]string{
api.ServingCertSecretAnnotation: testSecretName,
Expand All @@ -362,9 +370,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
api.ServingCertCreatedByAnnotation: signerName,
},
expectedSecretAnnotations: map[string]string{
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
updateService: true,
updateSecret: true,
Expand All @@ -386,9 +395,10 @@ func TestServiceServingCertControllerSync(t *testing.T) {
api.ServingCertCreatedByAnnotation: signerName,
},
expectedSecretAnnotations: map[string]string{
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
api.ServiceUIDAnnotation: testServiceUID,
api.ServiceNameAnnotation: testServiceName,
apiannotations.OpenShiftComponent: api.OwningJiraComponent,
apiannotations.OpenShiftDescription: descAnnotation,
},
updateService: true,
updateSecret: true,
Expand Down
Expand Up @@ -16,7 +16,6 @@ import (
listers "k8s.io/client-go/listers/core/v1"
"k8s.io/klog/v2"

apiannotations "github.com/openshift/api/annotations"
ocontroller "github.com/openshift/library-go/pkg/controller"
"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/crypto"
Expand Down Expand Up @@ -210,8 +209,8 @@ func (sc *serviceServingCertUpdateController) ensureSecretData(service *v1.Servi
}

// set the owning-component unless someone else has claimed it.
if !update && len(secretCopy.Annotations[apiannotations.OpenShiftComponent]) == 0 {
secretCopy.Annotations[apiannotations.OpenShiftComponent] = api.OwningJiraComponent
needsOwnerUpdate := setSecretOwnerDescription(secretCopy, service)
if !update && needsOwnerUpdate {
update = true
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/operator/v4_00_assets/bindata.go

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

0 comments on commit 5c0ebfe

Please sign in to comment.