chore: Create feature parity between cert-manager certificaterequest …

…and command-issuer, update docs

CHANGELOG.md

@@ -0,0 +1,9 @@
+# v1.0.4
+
+## Features
+* feat(signer): Signer recognizes `metadata.command-issuer.keyfactor.com/<metadata-field-name>: <metadata-value>` annotations on the CertificateRequest resource and uses them to populate certificate metadata in Command.
+* feat(release): Container build and release now uses GitHub Actions.
+* fix(helm): CRDs now correspond to correct values for the `command-issuer`.
+* fix(helm): Signer Helm Chart now includes a `secureMetrics` value to enable/disable sidecar RBAC container for further protection of the `/metrics` endpoint.
+* fix(signer): Signer now returns CA chain bytes instead of appending to the leaf certificate.
+* fix(role): Removed permissions for `configmaps` resource types for the `leader-election-role` role.

README.md

@@ -517,12 +517,6 @@ cat <<EOF >> metadata.json
             "Description": "The group name of the resource that the Issuer or ClusterIssuer controller is managing.",
             "Name": "Controller-Resource-Group-Name"
         },
-        {
-            "AllowAPI": true,
-            "DataType": 1,
-            "Description": "The name of the K8s issuer resource",
-            "Name": "Issuer-Name"
-        },
         {
             "AllowAPI": true,
             "DataType": 1,
@@ -540,6 +534,7 @@ cat <<EOF >> metadata.json
     "CustomReports": [],
     "SecurityRoles": []
 }
+EOF
 kfutil import --metadata --file metadata.json
 ```
 

config/manager/kustomization.yaml

@@ -5,4 +5,4 @@ kind: Kustomization
 images:
 - name: controller
   newName: ghcr.io/keyfactor/command-cert-manager-issuer
-  newTag: v1.0.4
+  newTag: latest

config/rbac/leader_election_role.yaml

@@ -11,18 +11,6 @@ metadata:
     app.kubernetes.io/managed-by: kustomize
   name: leader-election-role
 rules:
-- apiGroups:
-  - ""
-  resources:
-  - configmaps
-  verbs:
-  - get
-  - list
-  - watch
-  - create
-  - update
-  - patch
-  - delete
 - apiGroups:
   - coordination.k8s.io
   resources:

internal/controllers/certificaterequest_controller.go

@@ -247,11 +247,12 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
 	meta.ControllerReconcileId = string(controller.ReconcileIDFromContext(ctx))
 	meta.CertificateSigningRequestNamespace = certificateRequest.Namespace
 
-	signed, err := commandSigner.Sign(ctx, certificateRequest.Spec.Request, meta)
+	leaf, chain, err := commandSigner.Sign(ctx, certificateRequest.Spec.Request, meta)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("%w: %v", errSignerSign, err)
 	}
-	certificateRequest.Status.Certificate = signed
+	certificateRequest.Status.Certificate = leaf
+	certificateRequest.Status.CA = chain
 
 	setReadyCondition(cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "Signed")
 	return ctrl.Result{}, nil

internal/controllers/certificaterequest_controller_test.go

@@ -51,8 +51,8 @@ type fakeSigner struct {
 	errSign error
 }
 
-func (o *fakeSigner) Sign(context.Context, []byte, signer.K8sMetadata) ([]byte, error) {
-	return []byte("fake signed certificate"), o.errSign
+func (o *fakeSigner) Sign(context.Context, []byte, signer.K8sMetadata) ([]byte, []byte, error) {
+	return []byte("fake signed certificate"), []byte("fake ca chain"), o.errSign
 }
 
 func TestCertificateRequestReconcile(t *testing.T) {

internal/issuer/signer/signer.go

@@ -63,7 +63,7 @@ type HealthCheckerBuilder func(context.Context, *commandissuer.IssuerSpec, map[s
 type CommandSignerBuilder func(context.Context, *commandissuer.IssuerSpec, map[string]string, map[string][]byte, map[string][]byte) (Signer, error)
 
 type Signer interface {
-	Sign(context.Context, []byte, K8sMetadata) ([]byte, error)
+	Sign(context.Context, []byte, K8sMetadata) ([]byte, []byte, error)
 }
 
 func CommandHealthCheckerFromIssuerAndSecretData(ctx context.Context, spec *commandissuer.IssuerSpec, authSecretData map[string][]byte, caSecretData map[string][]byte) (HealthChecker, error) {
@@ -169,13 +169,13 @@ func (s *commandSigner) Check() error {
 	return errors.New("missing \"POST /Enrollment/CSR\" endpoint")
 }
 
-func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMetadata) ([]byte, error) {
+func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMetadata) ([]byte, []byte, error) {
 	k8sLog := log.FromContext(ctx)
 
 	csr, err := parseCSR(csrBytes)
 	if err != nil {
 		k8sLog.Error(err, "failed to parse CSR")
-		return nil, err
+		return nil, nil, err
 	}
 
 	// Log the common metadata of the CSR
@@ -241,12 +241,12 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe
 
 		k8sLog.Error(err, detail)
 
-		return nil, fmt.Errorf(detail)
+		return nil, nil, fmt.Errorf(detail)
 	}
 
 	certAndChain, err := getCertificatesFromCertificateInformation(commandCsrResponseObject.CertificateInformation)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	k8sLog.Info(fmt.Sprintf("Successfully enrolled certificate with Command with subject %q. Certificate has %d SANs", certAndChain[0].Subject, len(certAndChain[0].DNSNames)+len(certAndChain[0].IPAddresses)+len(certAndChain[0].URIs)))
@@ -277,20 +277,31 @@ func getCertificatesFromCertificateInformation(commandResp *keyfactor.ModelsPkcs
 
 // compileCertificatesToPemString takes a slice of x509 certificates and returns a string containing the certificates in PEM format
 // If an error occurred, the function logs the error and continues to parse the remaining objects.
-func compileCertificatesToPemBytes(certificates []*x509.Certificate) ([]byte, error) {
-	var pemBuilder strings.Builder
-
-	for _, certificate := range certificates {
-		err := pem.Encode(&pemBuilder, &pem.Block{
-			Type:  "CERTIFICATE",
-			Bytes: certificate.Raw,
-		})
-		if err != nil {
-			return make([]byte, 0), err
+func compileCertificatesToPemBytes(certificates []*x509.Certificate) ([]byte, []byte, error) {
+	var leaf strings.Builder
+	var chain strings.Builder
+
+	for i, certificate := range certificates {
+		if i == 0 {
+			err := pem.Encode(&leaf, &pem.Block{
+				Type:  "CERTIFICATE",
+				Bytes: certificate.Raw,
+			})
+			if err != nil {
+				return make([]byte, 0), make([]byte, 0), err
+			}
+		} else {
+			err := pem.Encode(&chain, &pem.Block{
+				Type:  "CERTIFICATE",
+				Bytes: certificate.Raw,
+			})
+			if err != nil {
+				return make([]byte, 0), make([]byte, 0), err
+			}
 		}
 	}
 
-	return []byte(pemBuilder.String()), nil
+	return []byte(leaf.String()), []byte(chain.String()), nil
 }
 
 const (

internal/issuer/signer/signer_test.go

@@ -86,13 +86,14 @@ func TestCommandSignerFromIssuerAndSecretData(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		signed, err := signer.Sign(context.Background(), csr, meta)
+		leaf, chain, err := signer.Sign(context.Background(), csr, meta)
 		if err != nil {
 			t.Fatal(err)
 		}
 		t.Logf("Signing took %s", time.Since(start))
 
-		t.Logf("Signed certificate: %s", string(signed))
+		t.Logf("Signed certificate: %s", string(leaf))
+		t.Logf("Chain: %s", string(chain))
 	})
 
 	// Set up test data
@@ -238,7 +239,7 @@ func TestCompileCertificatesToPemBytes(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := compileCertificatesToPemBytes(tt.certificates)
+			_, _, err = compileCertificatesToPemBytes(tt.certificates)
 			if (err != nil) != tt.expectedError {
 				t.Errorf("expected error = %v, got %v", tt.expectedError, err)
 			}
@@ -305,12 +306,7 @@ func Test_createCommandClientFromSecretData(t *testing.T) {
 		t.Fatalf("failed to generate self-signed certificate: %v", err)
 	}
 
-	cert2, err := generateSelfSignedCertificate()
-	if err != nil {
-		t.Fatalf("failed to generate self-signed certificate: %v", err)
-	}
-
-	certBytes, err := compileCertificatesToPemBytes([]*x509.Certificate{cert1, cert2})
+	leafBytes, _, err := compileCertificatesToPemBytes([]*x509.Certificate{cert1})
 	if err != nil {
 		return
 	}
@@ -377,7 +373,7 @@ func Test_createCommandClientFromSecretData(t *testing.T) {
 				"password": []byte("password"),
 			},
 			caSecretData: map[string][]byte{
-				"tls.crt": certBytes,
+				"tls.crt": leafBytes,
 			},
 			verify: func(t *testing.T, client *keyfactor.APIClient) error {
 				if client == nil {