Add ability to specify keyUsages

Up till now, the standard keyUsages were sufficient, but libvirt needs clientAuth. This allows us to specify the keyUsage in the service spec, instead of hard-coding it. Also modified the kuttl test to query the Certificate objects created.
openstack-k8s-operators · Mar 8, 2024 · bca6170 · bca6170
1 parent 04387ad
commit bca6170
Show file tree

Hide file tree

Showing 13 changed files with 216 additions and 19 deletions.
diff --git a/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml
@@ -60,6 +60,34 @@ spec:
                     type: array
                   issuer:
                     type: string
+                  keyUsages:
+                    items:
+                      enum:
+                      - signing
+                      - digital signature
+                      - content commitment
+                      - key encipherment
+                      - key agreement
+                      - data encipherment
+                      - cert sign
+                      - crl sign
+                      - encipher only
+                      - decipher only
+                      - any
+                      - server auth
+                      - client auth
+                      - code signing
+                      - email protection
+                      - s/mime
+                      - ipsec end system
+                      - ipsec tunnel
+                      - ipsec user
+                      - timestamping
+                      - ocsp signing
+                      - microsoft sgc
+                      - netscape sgc
+                      type: string
+                    type: array
                   networks:
                     items:
                       pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$

diff --git a/api/go.mod b/api/go.mod
@@ -12,7 +12,10 @@ require (
 	sigs.k8s.io/controller-runtime v0.16.5
 )
 
-require golang.org/x/exp v0.0.0-20240213143201-ec583247a57a
+require (
+	github.com/cert-manager/cert-manager v1.13.4
+	golang.org/x/exp v0.0.0-20240213143201-ec583247a57a
+)
 
 require (
 	github.com/beorn7/perks v1.0.1 // indirect
@@ -44,12 +47,10 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
-	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/client_golang v1.16.0 // indirect
 	github.com/prometheus/client_model v0.4.0 // indirect
 	github.com/prometheus/common v0.44.0 // indirect
 	github.com/prometheus/procfs v0.10.1 // indirect
-	github.com/rogpeppe/go-internal v1.11.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	go.uber.org/goleak v1.3.0 // indirect
 	golang.org/x/net v0.21.0 // indirect
@@ -70,6 +71,7 @@ require (
 	k8s.io/klog/v2 v2.110.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20230905202853-d090da108d2f // indirect
 	k8s.io/utils v0.0.0-20240102154912-e7106e64919e // indirect
+	sigs.k8s.io/gateway-api v0.8.0 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 	sigs.k8s.io/yaml v1.4.0 // indirect

diff --git a/api/go.sum b/api/go.sum
@@ -1,5 +1,7 @@
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
+github.com/cert-manager/cert-manager v1.13.4 h1:4zJdlemXg84KFssuk4I781oBJo1CuAnD1m8ZF/zsRrY=
+github.com/cert-manager/cert-manager v1.13.4/go.mod h1:8F9nXyWuOP0Ziq77g0N5N/sTyfP1NBVs4C1GBjrDU1I=
 github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
 github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
@@ -92,7 +94,6 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
-github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_golang v1.16.0 h1:yk/hx9hDbrGHovbci4BY+pRMfSuuat626eFsHb7tmT8=
 github.com/prometheus/client_golang v1.16.0/go.mod h1:Zsulrv/L9oM40tJ7T815tM89lFEugiJ9HzIqaAx4LKc=
 github.com/prometheus/client_model v0.4.0 h1:5lQXD3cAg1OXBf4Wq03gTrXHeaV0TQvGfUooCfx1yqY=
@@ -102,7 +103,6 @@ github.com/prometheus/common v0.44.0/go.mod h1:ofAIvZbQ1e/nugmZGz4/qCb9Ap1VoSTIO
 github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg=
 github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM=
 github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M=
-github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
@@ -200,6 +200,8 @@ k8s.io/utils v0.0.0-20240102154912-e7106e64919e h1:eQ/4ljkx21sObifjzXwlPKpdGLrCf
 k8s.io/utils v0.0.0-20240102154912-e7106e64919e/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 sigs.k8s.io/controller-runtime v0.16.5 h1:yr1cEJbX08xsTW6XEIzT13KHHmIyX8Umvme2cULvFZw=
 sigs.k8s.io/controller-runtime v0.16.5/go.mod h1:j7bialYoSn142nv9sCOJmQgDXQXxnroFU4VnX/brVJ0=
+sigs.k8s.io/gateway-api v0.8.0 h1:isQQ3Jx2qFP7vaA3ls0846F0Amp9Eq14P08xbSwVbQg=
+sigs.k8s.io/gateway-api v0.8.0/go.mod h1:okOnjPNBFbIS/Rw9kAhuIUaIkLhTKEu+ARIuXk2dgaM=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=

diff --git a/api/v1beta1/openstackdataplaneservice_types.go b/api/v1beta1/openstackdataplaneservice_types.go
@@ -19,6 +19,7 @@ package v1beta1
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 )
@@ -39,6 +40,10 @@ type OpenstackDataPlaneServiceCert struct {
 	// Only one issuer should have this label
 	// +kubebuilder:validation:Optional
 	Issuer string `json:"issuer,omitempty"`
+
+	// KeyUsages to be added to the issued cert
+	// +kubebuilder:validation:Optional
+	KeyUsages []certmgrv1.KeyUsage `json:"keyUsages,omitempty" yaml:"keyUsages,omitempty"`
 }
 
 // OpenStackDataPlaneServiceSpec defines the desired state of OpenStackDataPlaneService

diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml
@@ -60,6 +60,34 @@ spec:
                     type: array
                   issuer:
                     type: string
+                  keyUsages:
+                    items:
+                      enum:
+                      - signing
+                      - digital signature
+                      - content commitment
+                      - key encipherment
+                      - key agreement
+                      - data encipherment
+                      - cert sign
+                      - crl sign
+                      - encipher only
+                      - decipher only
+                      - any
+                      - server auth
+                      - client auth
+                      - code signing
+                      - email protection
+                      - s/mime
+                      - ipsec end system
+                      - ipsec tunnel
+                      - ipsec user
+                      - timestamping
+                      - ocsp signing
+                      - microsoft sgc
+                      - netscape sgc
+                      type: string
+                    type: array
                   networks:
                     items:
                       pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$

diff --git a/config/services/dataplane_v1beta1_openstackdataplaneservice_libvirt.yaml b/config/services/dataplane_v1beta1_openstackdataplaneservice_libvirt.yaml
@@ -10,5 +10,10 @@ spec:
       - ips
     networks:
       - ctlplane
+    keyUsages:
+      - digital signature
+      - key encipherment
+      - server auth
+      - client auth
     issuer: osp-rootca-issuer-internal
   caCerts: combined-ca-bundle
diff --git a/docs/assemblies/custom_resources.adoc b/docs/assemblies/custom_resources.adoc
@@ -349,6 +349,11 @@ OpenstackDataPlaneServiceCert defines the property of a TLS cert issued for a da
 | Issuer is the label for the issuer to issue the cert Only one issuer should have this label
 | string
 | false
+
+| keyUsages
+| KeyUsages to be added to the issued cert
+| []certmgrv1.KeyUsage
+| false
 |===
 
 <<custom-resources,Back to Custom Resources>>

diff --git a/docs/assemblies/tls.adoc b/docs/assemblies/tls.adoc
@@ -57,6 +57,11 @@ spec:
       - ips
     networks:
       - CtlPlane
+    keyUsages:
+      - digital signature
+      - key encipherment
+      - server auth
+      - client auth
     issuer: osp-rootca-issuer-internal
   caCerts: combined-ca-bundle
 ----
@@ -113,6 +118,14 @@ If more than one issuer has the label, an error is generated. If the issuers att
 configuration for `service1`, the certificates are issued with the default root CA for internal TLS as defined
 in `lib-common`, which is set to the label "osp-rootca-issuer-internal" for the `rootca-internal` issuer.
 
+==== keyUsages
+
+This attribute is a list of key uages to be included as key usage extensions in the certificate.  The
+strings that correspond to valid usages are provided by the https://github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1/types.go[certmanage api].
+If this attribute is not provided, the default set of key usages as defined in https://github.com/openstack-k8s-operators/lib-common/blob/main/modules/certmanager/certificate.go[lib-common].
+will be used.  These are "key encipherment", "digital signature" and "server auth".  In the above examples, we
+see that libvirt defines this attribute because the "client auth" key usage is also needed.
+
 === addCertMounts
 
 This attribute specifies whether or not to mount the certificates and keys generated for all

diff --git a/pkg/deployment/cert.go b/pkg/deployment/cert.go
@@ -123,12 +123,7 @@ func EnsureTLSCerts(ctx context.Context, helper *helper.Helper,
 
 		// TODO: paramaterize usage
 		certSecret, result, err = GetTLSNodeCert(ctx, helper, instance, certName,
-			issuer.Name, labels, hosts, ips, []certmgrv1.KeyUsage{
-				certmgrv1.UsageDigitalSignature,
-				certmgrv1.UsageKeyEncipherment,
-				certmgrv1.UsageServerAuth,
-				certmgrv1.UsageClientAuth,
-			})
+			issuer.Name, labels, hosts, ips, service.Spec.TLSCert.KeyUsages)
 
 		// handle cert request errors
 		if (err != nil) || (result != ctrl.Result{}) {

diff --git a/tests/kuttl/tests/dataplane-deploy-tls-test/02-assert.yaml b/tests/kuttl/tests/dataplane-deploy-tls-test/02-assert.yaml
@@ -13,19 +13,57 @@ metadata:
     osdpns: openstack-edpm-tls
 type: kubernetes.io/tls
 ---
-# validate the alt-names - which is a list with elements that can be in any order
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  labels:
+    hostname: edpm-compute-0
+    osdp-service: default-service-tls-dnsnames
+    osdpns: openstack-edpm-tls
+  name: default-service-tls-dnsnames-edpm-compute-0
+  namespace: openstack
+  ownerReferences:
+  - apiVersion: dataplane.openstack.org/v1beta1
+    kind: OpenStackDataPlaneDeployment
+    name: openstack-edpm-tls-deployment
+spec:
+  issuerRef:
+    group: cert-manager.io
+    kind: Issuer
+    name: rootca-internal
+  secretName: cert-default-service-tls-dnsnames-edpm-compute-0
+  secretTemplate:
+    labels:
+      hostname: edpm-compute-0
+      osdp-service: default-service-tls-dnsnames
+      osdpns: openstack-edpm-tls
+---
+# validate the alt-names and usages - which is a list with elements that can be in any order
 apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 commands:
   - script: |
-      template='{{index .metadata.annotations "cert-manager.io/alt-names" }}'
-      names=$(oc get secret cert-default-service-tls-dnsnames-edpm-compute-0 -n openstack -o go-template="$template")
+      template='{{index .spec.dnsNames }}'
+      names=$(oc get certificate default-service-tls-dnsnames-edpm-compute-0 -n openstack -o go-template="$template")
       echo $names > test123.data
       regex="(?=.*(edpm-compute-0\.internalapi\.example\.com))(?=.*(edpm-compute-0\.storage\.example\.com))(?=.*(edpm-compute-0\.tenant\.example\.com))(?=.*(edpm-compute-0\.ctlplane\.example\.com))"
       matches=$(grep -P "$regex" test123.data)
       rm test123.data
       if [ -z "$matches" ]; then
-         echo "bad match: $names"
+         echo "bad dnsnames match: $names"
+         exit 1
+      else
+         exit 0
+      fi
+  - script: |
+      template='{{index .spec.usages }}'
+      usages=$(oc get certificate default-service-tls-dnsnames-edpm-compute-0 -n openstack -o go-template="$template")
+      echo $usages > test123.data
+      regex="(?=.*(key encipherment))(?=.*(digital signature))(?=.*(server auth))"
+      matches=$(grep -P "$regex" test123.data)
+      rm test123.data
+      if [ -z "$matches" ]; then
+         echo "bad usages match: $usages"
          exit 1
       else
          exit 0

diff --git a/tests/kuttl/tests/dataplane-deploy-tls-test/03-assert.yaml b/tests/kuttl/tests/dataplane-deploy-tls-test/03-assert.yaml
@@ -15,6 +15,33 @@ metadata:
     osdpns: openstack-edpm-tls
 type: kubernetes.io/tls
 ---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  labels:
+    hostname: edpm-compute-0
+    osdp-service: custom-service-tls-dns-ips
+    osdpns: openstack-edpm-tls
+  name: custom-service-tls-dns-ips-edpm-compute-0
+  namespace: openstack
+  ownerReferences:
+  - apiVersion: dataplane.openstack.org/v1beta1
+    kind: OpenStackDataPlaneDeployment
+    name: openstack-edpm-tls-services-override
+spec:
+  dnsNames:
+  - edpm-compute-0.ctlplane.example.com
+  issuerRef:
+    group: cert-manager.io
+    kind: Issuer
+    name: rootca-internal
+  secretName: cert-custom-service-tls-dns-ips-edpm-compute-0
+  secretTemplate:
+    labels:
+      hostname: edpm-compute-0
+      osdp-service: custom-service-tls-dns-ips
+      osdpns: openstack-edpm-tls
+---
 apiVersion: v1
 kind: Secret
 metadata:
@@ -30,19 +57,57 @@ metadata:
     osdpns: openstack-edpm-tls
 type: kubernetes.io/tls
 ---
-# validate the alt-names - which is a list with elements that can be in any order
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  labels:
+    hostname: edpm-compute-0
+    osdp-service: custom-service-tls-dns
+    osdpns: openstack-edpm-tls
+  name: custom-service-tls-dns-edpm-compute-0
+  namespace: openstack
+  ownerReferences:
+  - apiVersion: dataplane.openstack.org/v1beta1
+    kind: OpenStackDataPlaneDeployment
+    name: openstack-edpm-tls-services-override
+spec:
+  issuerRef:
+    group: cert-manager.io
+    kind: Issuer
+    name: rootca-internal
+  secretName: cert-custom-service-tls-dns-edpm-compute-0
+  secretTemplate:
+    labels:
+      hostname: edpm-compute-0
+      osdp-service: custom-service-tls-dns
+      osdpns: openstack-edpm-tls
+---
+# validate the alt-names and usages - which is a list with elements that can be in any order
 apiVersion: kuttl.dev/v1beta1
 kind: TestAssert
 commands:
   - script: |
-      template='{{index .metadata.annotations "cert-manager.io/alt-names" }}'
-      names=$(oc get secret cert-custom-service-tls-dns-edpm-compute-0 -n openstack -o go-template="$template")
+      template='{{index .spec.dnsNames }}'
+      names=$(oc get certificate custom-service-tls-dns-edpm-compute-0 -n openstack -o go-template="$template")
       echo $names > test123.data
       regex="(?=.*(edpm-compute-0\.internalapi\.example\.com))(?=.*(edpm-compute-0\.storage\.example\.com))(?=.*(edpm-compute-0\.tenant\.example\.com))(?=.*(edpm-compute-0\.ctlplane\.example\.com))"
       matches=$(grep -P "$regex" test123.data)
       rm test123.data
       if [ -z "$matches" ]; then
-         echo "bad match: $names"
+         echo "bad dnsnames match: $names"
+         exit 1
+      else
+         exit 0
+      fi
+  - script: |
+      template='{{index .spec.usages }}'
+      usages=$(oc get certificate custom-service-tls-dns-edpm-compute-0 -n openstack -o go-template="$template")
+      echo $usages > test123.data
+      regex="(?=.*(key encipherment))(?=.*(digital signature))(?=.*(server auth))(?=.*(client auth))"
+      matches=$(grep -P "$regex" test123.data)
+      rm test123.data
+      if [ -z "$matches" ]; then
+         echo "bad usages match: $usages"
          exit 1
       else
          exit 0