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
dinhxuanvu committed Nov 14, 2023
1 parent fe88761 commit edc7c2b
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 37 deletions.
1 change: 1 addition & 0 deletions bindata/v4.0.0/controller/signing-cabundle.yaml
View file
Expand Up @@ -5,5 +5,6 @@ metadata:
name: signing-cabundle
annotations:
openshift.io/owning-component: service-ca
openshift.io/description: "Service CA signing CA bundle"
data:
ca-bundle.crt:
1 change: 1 addition & 0 deletions bindata/v4.0.0/controller/signing-secret.yaml
View file
Expand Up @@ -5,6 +5,7 @@ metadata:
name: signing-key
annotations:
openshift.io/owning-component: service-ca
openshift.io/description: "Service CA signing key"
type: kubernetes.io/tls
data:
tls.crt:
Expand Down
35 changes: 28 additions & 7 deletions pkg/controller/servingcert/controller/secret_creating_controller.go
View file
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,26 @@ 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.OpenShiftDescription]) == 0 {
secret.Annotations[apiannotations.OpenShiftComponent] = api.OwningJiraComponent
changed = true
}

// Copy description annotation from service to secret if exists. Otherwise, generate
// a description for it <service name>/<service namespace>
if len(secret.Annotations[apiannotations.OpenShiftDescription]) == 0 {
secret.Annotations[apiannotations.OpenShiftDescription] = fmt.Sprintf("Generated by Service CA for %s/%s", service.Name, service.Namespace)
changed = true
}
return changed
}
64 changes: 37 additions & 27 deletions pkg/controller/servingcert/controller/secret_creating_controller_test.go
View file
Expand Up @@ -37,6 +37,7 @@ const (
testServiceName = "svc-name"
testNamespace = "svc-namespace"
testSecretName = "new-secret"
descAnnotation = "Generated by Service CA for svc-name/svc-namespace"
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
4 changes: 1 addition & 3 deletions pkg/controller/servingcert/controller/secret_updating_controller.go
View file
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,7 @@ 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
if !update && setSecretOwnerDescription(secretCopy, service) {
update = true
}

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

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

0 comments on commit edc7c2b

Please sign in to comment.