From 52e013ae28fc1365acbfbe79d01f3e3ece7b8f64 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Fri, 6 Oct 2023 12:04:23 -0700 Subject: [PATCH 1/4] feat(metadata): Signer recognizes annotation prefix for Command metadata --- .github/workflows/release.yml | 190 ++++++++- Makefile | 14 +- README.md | 12 +- config/manager/kustomization.yaml | 4 +- .../templates/role.yaml | 12 - .../command-cert-manager-issuer/values.yaml | 2 +- internal/issuer/signer/signer.go | 43 +- internal/issuer/signer/signer_test.go | 383 +++++++++++++++++- 8 files changed, 603 insertions(+), 57 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index cb6bcd6..6a337b0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,43 +1,201 @@ -name: helm_release +name: Build and Release on: + push: + branches: + - '*' pull_request: branches: - 'v*' types: + # action should run when the pull request is closed + # (regardless of whether it was merged or just closed) - closed + # Make sure the action runs every time new commits are + # pushed to the pull request's branch + - synchronize + +env: + REGISTRY: ghcr.io + IMAGE_NAME: ${{ github.repository }} + jobs: + build: + name: Build Containers + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + platform: + - linux/arm64 + - linux/amd64 + - linux/s390x + - linux/ppc64le + + permissions: + contents: read + packages: write + + steps: + # Checkout code + # https://github.com/actions/checkout + - name: Checkout code + uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + + # Extract metadata (tags, labels) for Docker + # https://github.com/docker/metadata-action + - name: Extract Docker metadata + id: meta + uses: docker/metadata-action@96383f45573cb7f253c731d3b3ab81c87ef81934 # v5.0.0 + with: + images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + + # Set up QEMU + # https://github.com/docker/setup-qemu-action + - name: Set up QEMU + uses: docker/setup-qemu-action@68827325e0b33c7199eb31dd4e31fbe9023e06e3 # v3.0.0 + + # Set up BuildKit Docker container builder to be able to build + # multi-platform images and export cache + # https://github.com/docker/setup-buildx-action + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@f95db51fddba0c2d1ec667646a06c2ce06100226 # v3.0.0 + + # Login to Docker registry + # https://github.com/docker/login-action + - name: Log into registry ${{ env.REGISTRY }} + uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # v3.0.0 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + # Build and push Docker image with Buildx + # https://github.com/docker/build-push-action + - name: Build and push Docker image + id: build + uses: docker/build-push-action@0565240e2d4ab88bba5387d719585280857ece09 # v5.0.0 + with: + context: . + platforms: ${{ matrix.platform }} + labels: ${{ steps.meta.outputs.labels }} + push: ${{ github.event.pull_request.merged == true }} + outputs: type=image,name=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }},push-by-digest=true,name-canonical=true + + # Export digest + - name: Export digest + if: github.event.pull_request.merged == true + run: | + mkdir -p /tmp/digests + digest="${{ steps.build.outputs.digest }}" + touch "/tmp/digests/${digest#sha256:}" + + # Upload digest + - name: Upload digest + if: github.event.pull_request.merged == true + uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3 + with: + name: digests + path: /tmp/digests/* + if-no-files-found: error + retention-days: 1 + + merge: + runs-on: ubuntu-latest + if: github.event.pull_request.merged == true + needs: + - build + steps: + # Download digests + # https://github.com/actions/download-artifact + - name: Download digests + uses: actions/download-artifact@9bc31d5ccc31df68ecc42ccf4149144866c47d8a # v3.0.2 + with: + name: digests + path: /tmp/digests + + # Set up BuildKit Docker container builder to be able to build + # multi-platform images and export cache + # https://github.com/docker/setup-buildx-action + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@f95db51fddba0c2d1ec667646a06c2ce06100226 # v3.0.0 + + # Extract metadata (tags, labels) for Docker + # https://github.com/docker/metadata-action + - name: Extract Docker metadata + id: meta + uses: docker/metadata-action@96383f45573cb7f253c731d3b3ab81c87ef81934 # v5.0.0 + with: + images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + + # Login to Docker registry + # https://github.com/docker/login-action + - name: Log into registry ${{ env.REGISTRY }} + uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # v3.0.0 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + # Create manifest list and push + - name: Create manifest list and push + working-directory: /tmp/digests + run: | + docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ + $(printf '${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@sha256:%s ' *) + + - name: Inspect image + run: | + docker buildx imagetools inspect ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.meta.outputs.version }} + helm: runs-on: ubuntu-latest if: github.event.pull_request.merged == true + needs: + - merge steps: - - name: Extract Version Tag - id: extract_version - run: /bin/bash -c 'echo ::set-output name=VERSION::$(echo ${GITHUB_REF##*/} | cut -c2-)' + # Checkout code + # https://github.com/actions/checkout + - name: Checkout code + uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 - - name: Checkout - uses: actions/checkout@v3 + # Extract metadata (tags, labels) to use in Helm chart + # https://github.com/docker/metadata-action + - name: Extract Docker metadata + id: meta + uses: docker/metadata-action@96383f45573cb7f253c731d3b3ab81c87ef81934 # v5.0.0 + with: + images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + + # Set version from DOCKER_METADATA_OUTPUT_VERSION as environment variable + - name: Set Version + run: | + echo "VERSION=${DOCKER_METADATA_OUTPUT_VERSION:1}" >> $GITHUB_ENV # Change version and appVersion in Chart.yaml to the tag in the closed PR - name: Update Helm App/Chart Version shell: bash run: | - sed -i "s/^version: .*/version: ${{ steps.extract_version.outputs.VERSION }}/g" deploy/charts/command-cert-manager-issuer/Chart.yaml - sed -i "s/^appVersion: .*/appVersion: \"${{ steps.extract_version.outputs.VERSION }}\"/g" deploy/charts/command-cert-manager-issuer/Chart.yaml + sed -i "s/^version: .*/version: ${{ env.VERSION }}/g" deploy/charts/command-cert-manager-issuer/Chart.yaml + sed -i "s/^appVersion: .*/appVersion: \"${{ env.DOCKER_METADATA_OUTPUT_VERSION }}\"/g" deploy/charts/command-cert-manager-issuer/Chart.yaml + # Setup Helm + # https://github.com/Azure/setup-helm + - name: Install Helm + uses: azure/setup-helm@5119fcb9089d432beecbf79bb2c7915207344b78 # v3.5 + with: + token: ${{ secrets.GITHUB_TOKEN }} + + # Helm requires an ident name to be set for chart-releaser to work - name: Configure Git run: | git config user.name "$GITHUB_ACTOR" git config user.email "$GITHUB_ACTOR@users.noreply.github.com" - - name: Install Helm - uses: azure/setup-helm@v3 - + # Build and release Helm chart to GitHub Pages + # https://github.com/helm/chart-releaser-action - name: Run chart-releaser - uses: helm/chart-releaser-action@v1.5.0 + uses: helm/chart-releaser-action@be16258da8010256c6e82849661221415f031968 # v1.5.0 env: CR_TOKEN: "${{ secrets.GITHUB_TOKEN }}" with: - pages_branch: gh-pages - charts_dir: deploy/charts - mark_as_latest: true - packages_with_index: true + charts_dir: deploy/charts \ No newline at end of file diff --git a/Makefile b/Makefile index e7ebbd4..fba1c67 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,9 @@ # The version which will be reported by the --version argument of each binary # and which will be used as the Docker image tag -VERSION ?= 1.0.3 +VERSION ?= v1.0.4 # The Docker repository name, overridden in CI. -DOCKER_REGISTRY ?= m8rmclarenkf -DOCKER_IMAGE_NAME ?= command-cert-manager-external-issuer-controller +DOCKER_REGISTRY ?= ghcr.io +DOCKER_IMAGE_NAME ?= keyfactor/command-cert-manager-issuer # Image URL to use all building/pushing image targets IMG ?= ${DOCKER_REGISTRY}/${DOCKER_IMAGE_NAME}:${VERSION} #IMG ?= command-issuer-dev:latest @@ -122,6 +122,14 @@ deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} $(KUSTOMIZE) build config/default | kubectl apply -f - +# Build the manager image for local development. This image is not intended to be used in production. +# Then, install it into the K8s cluster +.PHONY: deploy-local +deploy-local: manifests kustomize ## Build docker image with the manager. + docker build -t ejbca-issuer-dev:latest -f Dockerfile . + cd config/manager && $(KUSTOMIZE) edit set image controller=ejbca-issuer-dev:latest + $(KUSTOMIZE) build config/default | kubectl apply -f - + .PHONY: undeploy undeploy: ## Undeploy controller from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion. $(KUSTOMIZE) build config/default | kubectl delete --ignore-not-found=$(ignore-not-found) -f - diff --git a/README.md b/README.md index 100542a..308428e 100644 --- a/README.md +++ b/README.md @@ -267,6 +267,15 @@ Here are the supported annotations that can override the default values: command-issuer.keyfactor.com/certificateAuthorityHostname: "example.com" ``` +### Metadata Annotations + +The Keyfactor Command external issuer for cert-manager also allows you to specify Command Metadata through the use of annotations. Metadata attached to a certificate request will be stored in Command and can be used for reporting and auditing purposes. The syntax for specifying metadata is as follows: +```yaml +metadata.command-issuer.keyfactor.com/: +``` + +###### :pushpin: The metadata field name must match a name of a metadata field in Command exactly. If the metadata field name does not match, the CSR enrollment will fail. + ### How to Apply Annotations To apply these annotations, include them in the metadata section of your CertificateRequest resource: @@ -278,9 +287,10 @@ metadata: annotations: command-issuer.keyfactor.com/certificateTemplate: "Ephemeral2day" command-issuer.keyfactor.com/certificateAuthorityLogicalName: "InternalIssuingCA1" + metadata.command-issuer.keyfactor.com/ResponsibleTeam: "theResponsibleTeam@example.com" # ... other annotations spec: -# ... rest of the spec +# ... the rest of the spec ``` ### Demo ClusterIssuer Usage with K8s Ingress diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 6badba7..3bcb896 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: m8rmclarenkf/command-external-issuer - newTag: v1.0.2 + newName: ghcr.io/keyfactor/command-cert-manager-issuer + newTag: v1.0.4 diff --git a/deploy/charts/command-cert-manager-issuer/templates/role.yaml b/deploy/charts/command-cert-manager-issuer/templates/role.yaml index bd3d437..78b9b28 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/role.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/role.yaml @@ -5,18 +5,6 @@ metadata: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} name: {{ include "command-cert-manager-issuer.name" . }}-leader-election-role rules: - - apiGroups: - - "" - resources: - - configmaps - verbs: - - get - - list - - watch - - create - - update - - patch - - delete - apiGroups: - coordination.k8s.io resources: diff --git a/deploy/charts/command-cert-manager-issuer/values.yaml b/deploy/charts/command-cert-manager-issuer/values.yaml index 876ea0b..9c14ccb 100644 --- a/deploy/charts/command-cert-manager-issuer/values.yaml +++ b/deploy/charts/command-cert-manager-issuer/values.yaml @@ -4,7 +4,7 @@ replicaCount: 1 image: - repository: m8rmclarenkf/command-cert-manager-external-issuer-controller + repository: ghcr.io/keyfactor/command-cert-manager-issuer pullPolicy: IfNotPresent # Overrides the image tag whose default is the chart appVersion. tag: "" diff --git a/internal/issuer/signer/signer.go b/internal/issuer/signer/signer.go index 1f70b4e..b926959 100644 --- a/internal/issuer/signer/signer.go +++ b/internal/issuer/signer/signer.go @@ -51,6 +51,7 @@ type commandSigner struct { certificateAuthorityLogicalName string certificateAuthorityHostname string certManagerCertificateName string + customMetadata map[string]interface{} } type HealthChecker interface { @@ -78,6 +79,10 @@ func CommandHealthCheckerFromIssuerAndSecretData(ctx context.Context, spec *comm } func CommandSignerFromIssuerAndSecretData(ctx context.Context, spec *commandissuer.IssuerSpec, annotations map[string]string, authSecretData map[string][]byte, caSecretData map[string][]byte) (Signer, error) { + return commandSignerFromIssuerAndSecretData(ctx, spec, annotations, authSecretData, caSecretData) +} + +func commandSignerFromIssuerAndSecretData(ctx context.Context, spec *commandissuer.IssuerSpec, annotations map[string]string, authSecretData map[string][]byte, caSecretData map[string][]byte) (*commandSigner, error) { k8sLog := log.FromContext(ctx) signer := commandSigner{} @@ -121,9 +126,23 @@ func CommandSignerFromIssuerAndSecretData(ctx context.Context, spec *commandissu k8sLog.Info(fmt.Sprintf("Using certificate template \"%s\" and certificate authority \"%s\" (%s)", signer.certificateTemplate, signer.certificateAuthorityLogicalName, signer.certificateAuthorityHostname)) + signer.customMetadata = extractMetadataFromAnnotations(annotations) + return &signer, nil } +func extractMetadataFromAnnotations(annotations map[string]string) map[string]interface{} { + metadata := make(map[string]interface{}) + + for key, value := range annotations { + if strings.HasPrefix(key, "metadata.command-issuer.keyfactor.com/") { + metadata[strings.TrimPrefix(key, "metadata.command-issuer.keyfactor.com/")] = value + } + } + + return metadata +} + func (s *commandSigner) Check() error { endpoints, _, err := s.client.StatusApi.StatusGetEndpoints(context.Background()).Execute() if err != nil { @@ -161,6 +180,19 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe // Log the common metadata of the CSR k8sLog.Info(fmt.Sprintf("Found CSR wtih Common Name \"%s\" and %d DNS SANs, %d IP SANs, and %d URI SANs", csr.Subject.CommonName, len(csr.DNSNames), len(csr.IPAddresses), len(csr.URIs))) + // Print the SANs + for _, dnsName := range csr.DNSNames { + k8sLog.Info(fmt.Sprintf("DNS SAN: %s", dnsName)) + } + + for _, ipAddress := range csr.IPAddresses { + k8sLog.Info(fmt.Sprintf("IP SAN: %s", ipAddress.String())) + } + + for _, uri := range csr.URIs { + k8sLog.Info(fmt.Sprintf("URI SAN: %s", uri.String())) + } + modelRequest := keyfactor.ModelsEnrollmentCSREnrollmentRequest{ CSR: string(csrBytes), IncludeChain: ptr(true), @@ -177,6 +209,11 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe SANs: nil, } + for metaName, value := range s.customMetadata { + k8sLog.Info(fmt.Sprintf("Adding metadata \"%s\" with value \"%s\"", metaName, value)) + modelRequest.Metadata[metaName] = value + } + var caBuilder strings.Builder if s.certificateAuthorityHostname != "" { caBuilder.WriteString(s.certificateAuthorityHostname) @@ -189,7 +226,11 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe commandCsrResponseObject, _, err := s.client.EnrollmentApi.EnrollmentPostCSREnroll(context.Background()).Request(modelRequest).XCertificateformat(enrollmentPEMFormat).Execute() if err != nil { - detail := fmt.Sprintf("error enrolling certificate with Command. verify that the certificate template \"%s\" exists and that the certificate authority \"%s\" (%s) is configured correctly", s.certificateTemplate, s.certificateAuthorityLogicalName, s.certificateAuthorityHostname) + detail := fmt.Sprintf("error enrolling certificate with Command. Verify that the certificate template \"%s\" exists and that the certificate authority \"%s\" (%s) is configured correctly.", s.certificateTemplate, s.certificateAuthorityLogicalName, s.certificateAuthorityHostname) + + if len(s.customMetadata) > 0 { + detail += " Also verify that the metadata fields provided exist in Command." + } var bodyError *keyfactor.GenericOpenAPIError ok := errors.As(err, &bodyError) diff --git a/internal/issuer/signer/signer_test.go b/internal/issuer/signer/signer_test.go index d2fc2ee..d7b8c25 100644 --- a/internal/issuer/signer/signer_test.go +++ b/internal/issuer/signer/signer_test.go @@ -27,7 +27,11 @@ import ( "encoding/pem" "fmt" commandissuer "github.com/Keyfactor/command-issuer/api/v1alpha1" + "github.com/Keyfactor/keyfactor-go-client-sdk/api/keyfactor" + "github.com/stretchr/testify/assert" + "math/big" "os" + "reflect" "strings" "testing" "time" @@ -55,39 +59,348 @@ func TestCommandHealthCheckerFromIssuerAndSecretData(t *testing.T) { } func TestCommandSignerFromIssuerAndSecretData(t *testing.T) { - obj := testSigner{ - SignerBuilder: CommandSignerFromIssuerAndSecretData, + t.Run("ValidSigning", func(t *testing.T) { + obj := testSigner{ + SignerBuilder: CommandSignerFromIssuerAndSecretData, + } + + // Generate a test CSR to sign + csr, err := generateCSR("C=US,ST=California,L=San Francisco,O=Keyfactor,OU=Engineering,CN=example.com") + if err != nil { + t.Fatal(err) + } + + meta := K8sMetadata{ + ControllerNamespace: "test-namespace", + ControllerKind: "Issuer", + ControllerResourceGroupName: "test-issuer.example.com", + IssuerName: "test-issuer", + IssuerNamespace: "test-namespace", + ControllerReconcileId: "GUID", + CertificateSigningRequestNamespace: "test-namespace", + } + + start := time.Now() + signer, err := obj.SignerBuilder(getTestSignerConfigItems(t)) + if err != nil { + t.Fatal(err) + } + + signed, 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)) + }) + + // Set up test data + + spec := commandissuer.IssuerSpec{ + Hostname: "example-hostname.com", + CertificateTemplate: "example-template", + CertificateAuthorityLogicalName: "example-logical-name", + CertificateAuthorityHostname: "ca-hostname.com", + SecretName: "example-secret-name", + CaSecretName: "example-ca-secret-name", + } + + authSecretData := map[string][]byte{ + "username": []byte("username"), + "password": []byte("password"), } - // Generate a test CSR to sign - csr, err := generateCSR("C=US,ST=California,L=San Francisco,O=Keyfactor,OU=Engineering,CN=example.com") + caSecretData := map[string][]byte{ + "tls.crt": []byte("ca-cert"), + } + + t.Run("MissingCertTemplate", func(t *testing.T) { + templateCopy := spec.CertificateTemplate + spec.CertificateTemplate = "" + // Create the signer + _, err := commandSignerFromIssuerAndSecretData(context.Background(), &spec, make(map[string]string), authSecretData, caSecretData) + if err == nil { + t.Errorf("expected error, got nil") + } + + spec.CertificateTemplate = templateCopy + }) + + t.Run("MissingCaLogicalName", func(t *testing.T) { + logicalNameCopy := spec.CertificateAuthorityLogicalName + spec.CertificateAuthorityLogicalName = "" + // Create the signer + _, err := commandSignerFromIssuerAndSecretData(context.Background(), &spec, make(map[string]string), authSecretData, caSecretData) + if err == nil { + t.Errorf("expected error, got nil") + } + + spec.CertificateAuthorityLogicalName = logicalNameCopy + }) + + t.Run("NoAnnotations", func(t *testing.T) { + // Create the signer + signer, err := commandSignerFromIssuerAndSecretData(context.Background(), &spec, make(map[string]string), authSecretData, caSecretData) + if err != nil { + t.Fatal(err) + } + + // If there are no annotations, the customMetadata map should be empty + if len(signer.customMetadata) != 0 { + t.Errorf("expected customMetadata to be empty, got %v", signer.customMetadata) + } + }) + + t.Run("MetadataAnnotations", func(t *testing.T) { + annotations := map[string]string{ + "metadata.command-issuer.keyfactor.com/key1": "value1", + "metadata.command-issuer.keyfactor.com/key2": "value2", + } + + // Create the signer + signer, err := commandSignerFromIssuerAndSecretData(context.Background(), &spec, annotations, authSecretData, caSecretData) + if err != nil { + t.Fatal(err) + } + + // If there are no annotations, the customMetadata map should be empty + if len(signer.customMetadata) != 2 { + t.Errorf("expected customMetadata to have 2 entries, got %v", signer.customMetadata) + } + + if value, ok := signer.customMetadata["key1"].(string); ok && value == "value1" { + // They are equal + } else { + t.Errorf("expected customMetadata key1 to be value1, got %v", signer.customMetadata["key1"]) + } + + if value, ok := signer.customMetadata["key2"].(string); ok && value == "value2" { + // They are equal + } else { + t.Errorf("expected customMetadata key1 to be value1, got %v", signer.customMetadata["key1"]) + } + }) + + t.Run("AnnotationDefaultOverrides", func(t *testing.T) { + annotations := map[string]string{ + "command-issuer.keyfactor.com/certificateTemplate": "TestCertificateTemplate", + "command-issuer.keyfactor.com/certificateAuthorityLogicalName": "TestCertificateAuthorityLogicalName", + "command-issuer.keyfactor.com/certificateAuthorityHostname": "TestCertificateAuthorityHostname", + "command-manager.io/certificate-name": "TestCertificateName", + } + + // Create the signer + signer, err := commandSignerFromIssuerAndSecretData(context.Background(), &spec, annotations, authSecretData, caSecretData) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, "TestCertificateTemplate", signer.certificateTemplate) + assert.Equal(t, "TestCertificateAuthorityLogicalName", signer.certificateAuthorityLogicalName) + assert.Equal(t, "TestCertificateAuthorityHostname", signer.certificateAuthorityHostname) + assert.Equal(t, "TestCertificateName", signer.certManagerCertificateName) + }) +} + +func TestCompileCertificatesToPemBytes(t *testing.T) { + // Generate two certificates for testing + cert1, err := generateSelfSignedCertificate() if err != nil { - t.Fatal(err) + t.Fatalf("failed to generate mock certificate: %v", err) + } + cert2, err := generateSelfSignedCertificate() + if err != nil { + t.Fatalf("failed to generate mock certificate: %v", err) + } + + tests := []struct { + name string + certificates []*x509.Certificate + expectedError bool + }{ + { + name: "No certificates", + certificates: []*x509.Certificate{}, + expectedError: false, + }, + { + name: "Single certificate", + certificates: []*x509.Certificate{cert1}, + expectedError: false, + }, + { + name: "Multiple certificates", + certificates: []*x509.Certificate{cert1, cert2}, + expectedError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := compileCertificatesToPemBytes(tt.certificates) + if (err != nil) != tt.expectedError { + t.Errorf("expected error = %v, got %v", tt.expectedError, err) + } + }) } +} - meta := K8sMetadata{ - ControllerNamespace: "test-namespace", - ControllerKind: "Issuer", - ControllerResourceGroupName: "test-issuer.example.com", - IssuerName: "test-issuer", - IssuerNamespace: "test-namespace", - ControllerReconcileId: "GUID", - CertificateSigningRequestNamespace: "test-namespace", +func Test_extractMetadataFromAnnotations(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected map[string]interface{} + }{ + { + name: "empty annotations", + annotations: map[string]string{}, + expected: map[string]interface{}{}, + }, + { + name: "annotations without metadata prefix", + annotations: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expected: map[string]interface{}{}, + }, + { + name: "annotations with metadata prefix", + annotations: map[string]string{ + "metadata.command-issuer.keyfactor.com/key1": "value1", + "key2": "value2", + }, + expected: map[string]interface{}{ + "key1": "value1", + }, + }, + { + name: "mixed annotations", + annotations: map[string]string{ + "metadata.command-issuer.keyfactor.com/key1": "value1", + "metadata.command-issuer.keyfactor.com/key2": "value2", + "key3": "value3", + }, + expected: map[string]interface{}{ + "key1": "value1", + "key2": "value2", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractMetadataFromAnnotations(tt.annotations) + if !reflect.DeepEqual(result, tt.expected) { + t.Errorf("expected %v, got %v", tt.expected, result) + } + }) } +} - start := time.Now() - signer, err := obj.SignerBuilder(getTestSignerConfigItems(t)) +func Test_createCommandClientFromSecretData(t *testing.T) { + cert1, err := generateSelfSignedCertificate() if err != nil { - t.Fatal(err) + t.Fatalf("failed to generate self-signed certificate: %v", err) } - signed, err := signer.Sign(context.Background(), csr, meta) + cert2, err := generateSelfSignedCertificate() if err != nil { - t.Fatal(err) + t.Fatalf("failed to generate self-signed certificate: %v", err) } - t.Logf("Signing took %s", time.Since(start)) - t.Logf("Signed certificate: %s", string(signed)) + certBytes, err := compileCertificatesToPemBytes([]*x509.Certificate{cert1, cert2}) + if err != nil { + return + } + + tests := []struct { + name string + spec commandissuer.IssuerSpec + authSecretData map[string][]byte + caSecretData map[string][]byte + verify func(*testing.T, *keyfactor.APIClient) error + expectedErr bool + }{ + { + name: "EmptySecretData", + authSecretData: map[string][]byte{ + "username": []byte(""), + "password": []byte(""), + }, + verify: func(t *testing.T, client *keyfactor.APIClient) error { + if client != nil { + return fmt.Errorf("expected client to be nil") + } + return nil + }, + expectedErr: true, + }, + { + name: "ValidAuthData", + spec: commandissuer.IssuerSpec{ + Hostname: "hostname", + }, + authSecretData: map[string][]byte{ + "username": []byte("username"), + "password": []byte("password"), + }, + verify: func(t *testing.T, client *keyfactor.APIClient) error { + if client == nil { + return fmt.Errorf("expected client to be non-nil") + } + + if client.GetConfig().Host != "hostname" { + return fmt.Errorf("expected hostname to be hostname, got %s", client.GetConfig().Host) + } + + if client.GetConfig().BasicAuth.UserName != "username" { + return fmt.Errorf("expected username to be username, got %s", client.GetConfig().BasicAuth.UserName) + } + + if client.GetConfig().BasicAuth.Password != "password" { + return fmt.Errorf("expected password to be password, got %s", client.GetConfig().BasicAuth.Password) + } + + return nil + }, + expectedErr: false, + }, + { + name: "InvalidCaData", + spec: commandissuer.IssuerSpec{ + Hostname: "hostname", + }, + authSecretData: map[string][]byte{ + "username": []byte("username"), + "password": []byte("password"), + }, + caSecretData: map[string][]byte{ + "tls.crt": certBytes, + }, + verify: func(t *testing.T, client *keyfactor.APIClient) error { + if client == nil { + return fmt.Errorf("expected client to be non-nil") + } + + return nil + }, + expectedErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := createCommandClientFromSecretData(context.Background(), &tt.spec, tt.authSecretData, tt.caSecretData) + if (err != nil) != tt.expectedErr { + t.Errorf("expected error = %v, got %v", tt.expectedErr, err) + } + if err = tt.verify(t, result); err != nil { + t.Error(err) + } + }) + } } func getTestHealthCheckerConfigItems(t *testing.T) (context.Context, *commandissuer.IssuerSpec, map[string][]byte, map[string][]byte) { @@ -142,7 +455,7 @@ func getTestSignerConfigItems(t *testing.T) (context.Context, *commandissuer.Iss // Read the CA cert from the file system. caCertBytes, err := os.ReadFile(pathToCaCert) if err != nil { - t.Log("CA cert not found, assuming that EJBCA is using a trusted CA") + t.Log("CA cert not found, assuming that Command is using a trusted CA") } caSecretData := map[string][]byte{} @@ -219,3 +532,31 @@ func parseSubjectDN(subject string, randomizeCn bool) (pkix.Name, error) { return name, nil } + +func generateSelfSignedCertificate() (*x509.Certificate, error) { + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, err + } + + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + BasicConstraintsValid: true, + } + + certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv) + if err != nil { + return nil, err + } + + cert, err := x509.ParseCertificate(certDER) + if err != nil { + return nil, err + } + + return cert, nil +} From c849c6408bb8eb70c7d4547af2b004794b36e763 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Fri, 6 Oct 2023 12:10:23 -0700 Subject: [PATCH 2/4] chore: Replace string format with --- internal/issuer/signer/signer.go | 17 +++++++++-------- internal/issuer/signer/signer_test.go | 14 +++++++------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/internal/issuer/signer/signer.go b/internal/issuer/signer/signer.go index b926959..a1846d3 100644 --- a/internal/issuer/signer/signer.go +++ b/internal/issuer/signer/signer.go @@ -32,7 +32,8 @@ import ( const ( // Keyfactor enrollment PEM format - enrollmentPEMFormat = "PEM" + enrollmentPEMFormat = "PEM" + commandMetadataAnnotationPrefix = "metadata.command-issuer.keyfactor.com/" ) type K8sMetadata struct { @@ -124,7 +125,7 @@ func commandSignerFromIssuerAndSecretData(ctx context.Context, spec *commandissu signer.certManagerCertificateName = value } - k8sLog.Info(fmt.Sprintf("Using certificate template \"%s\" and certificate authority \"%s\" (%s)", signer.certificateTemplate, signer.certificateAuthorityLogicalName, signer.certificateAuthorityHostname)) + k8sLog.Info(fmt.Sprintf("Using certificate template %q and certificate authority %q (%s)", signer.certificateTemplate, signer.certificateAuthorityLogicalName, signer.certificateAuthorityHostname)) signer.customMetadata = extractMetadataFromAnnotations(annotations) @@ -135,8 +136,8 @@ func extractMetadataFromAnnotations(annotations map[string]string) map[string]in metadata := make(map[string]interface{}) for key, value := range annotations { - if strings.HasPrefix(key, "metadata.command-issuer.keyfactor.com/") { - metadata[strings.TrimPrefix(key, "metadata.command-issuer.keyfactor.com/")] = value + if strings.HasPrefix(key, commandMetadataAnnotationPrefix) { + metadata[strings.TrimPrefix(key, commandMetadataAnnotationPrefix)] = value } } @@ -178,7 +179,7 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe } // Log the common metadata of the CSR - k8sLog.Info(fmt.Sprintf("Found CSR wtih Common Name \"%s\" and %d DNS SANs, %d IP SANs, and %d URI SANs", csr.Subject.CommonName, len(csr.DNSNames), len(csr.IPAddresses), len(csr.URIs))) + k8sLog.Info(fmt.Sprintf("Found CSR wtih Common Name %q and %d DNS SANs, %d IP SANs, and %d URI SANs", csr.Subject.CommonName, len(csr.DNSNames), len(csr.IPAddresses), len(csr.URIs))) // Print the SANs for _, dnsName := range csr.DNSNames { @@ -210,7 +211,7 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe } for metaName, value := range s.customMetadata { - k8sLog.Info(fmt.Sprintf("Adding metadata \"%s\" with value \"%s\"", metaName, value)) + k8sLog.Info(fmt.Sprintf("Adding metadata %q with value %q", metaName, value)) modelRequest.Metadata[metaName] = value } @@ -226,7 +227,7 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe commandCsrResponseObject, _, err := s.client.EnrollmentApi.EnrollmentPostCSREnroll(context.Background()).Request(modelRequest).XCertificateformat(enrollmentPEMFormat).Execute() if err != nil { - detail := fmt.Sprintf("error enrolling certificate with Command. Verify that the certificate template \"%s\" exists and that the certificate authority \"%s\" (%s) is configured correctly.", s.certificateTemplate, s.certificateAuthorityLogicalName, s.certificateAuthorityHostname) + detail := fmt.Sprintf("error enrolling certificate with Command. Verify that the certificate template %q exists and that the certificate authority %q (%s) is configured correctly.", s.certificateTemplate, s.certificateAuthorityLogicalName, s.certificateAuthorityHostname) if len(s.customMetadata) > 0 { detail += " Also verify that the metadata fields provided exist in Command." @@ -248,7 +249,7 @@ func (s *commandSigner) Sign(ctx context.Context, csrBytes []byte, k8sMeta K8sMe return nil, err } - k8sLog.Info(fmt.Sprintf("Successfully enrolled certificate with Command with subject \"%s\". Certificate has %d SANs", certAndChain[0].Subject, len(certAndChain[0].DNSNames)+len(certAndChain[0].IPAddresses)+len(certAndChain[0].URIs))) + 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))) // Return the certificate and chain in PEM format return compileCertificatesToPemBytes(certAndChain) diff --git a/internal/issuer/signer/signer_test.go b/internal/issuer/signer/signer_test.go index d7b8c25..232e506 100644 --- a/internal/issuer/signer/signer_test.go +++ b/internal/issuer/signer/signer_test.go @@ -154,8 +154,8 @@ func TestCommandSignerFromIssuerAndSecretData(t *testing.T) { t.Run("MetadataAnnotations", func(t *testing.T) { annotations := map[string]string{ - "metadata.command-issuer.keyfactor.com/key1": "value1", - "metadata.command-issuer.keyfactor.com/key2": "value2", + commandMetadataAnnotationPrefix + "key1": "value1", + commandMetadataAnnotationPrefix + "key2": "value2", } // Create the signer @@ -268,8 +268,8 @@ func Test_extractMetadataFromAnnotations(t *testing.T) { { name: "annotations with metadata prefix", annotations: map[string]string{ - "metadata.command-issuer.keyfactor.com/key1": "value1", - "key2": "value2", + commandMetadataAnnotationPrefix + "key1": "value1", + "key2": "value2", }, expected: map[string]interface{}{ "key1": "value1", @@ -278,9 +278,9 @@ func Test_extractMetadataFromAnnotations(t *testing.T) { { name: "mixed annotations", annotations: map[string]string{ - "metadata.command-issuer.keyfactor.com/key1": "value1", - "metadata.command-issuer.keyfactor.com/key2": "value2", - "key3": "value3", + commandMetadataAnnotationPrefix + "key1": "value1", + commandMetadataAnnotationPrefix + "key2": "value2", + "key3": "value3", }, expected: map[string]interface{}{ "key1": "value1", From ac412faf7732bf56a180c4bde7730769ab7d27b6 Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Fri, 6 Oct 2023 12:25:55 -0700 Subject: [PATCH 3/4] bug(release): Force org/repository inside to lowercase --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6a337b0..ebc2ad5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,7 +16,7 @@ on: env: REGISTRY: ghcr.io - IMAGE_NAME: ${{ github.repository }} + IMAGE_NAME: ${{ github.repository }}.toLowerCase() jobs: build: From 5a8369d5c5e80057b07eb5f6ec40d6aa79980b9a Mon Sep 17 00:00:00 2001 From: Hayden Roszell Date: Fri, 6 Oct 2023 13:13:20 -0700 Subject: [PATCH 4/4] bug(release): Force org/repository inside to lowercase with parameter expansion --- .github/workflows/release.yml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ebc2ad5..65f622a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,7 +16,6 @@ on: env: REGISTRY: ghcr.io - IMAGE_NAME: ${{ github.repository }}.toLowerCase() jobs: build: @@ -36,6 +35,11 @@ jobs: packages: write steps: + + - name: Set IMAGE_NAME + run: | + echo "IMAGE_NAME=${GITHUB_REPOSITORY,,}" >>${GITHUB_ENV} + # Checkout code # https://github.com/actions/checkout - name: Checkout code @@ -105,6 +109,10 @@ jobs: needs: - build steps: + - name: Set IMAGE_NAME + run: | + echo "IMAGE_NAME=${GITHUB_REPOSITORY,,}" >>${GITHUB_ENV} + # Download digests # https://github.com/actions/download-artifact - name: Download digests @@ -153,6 +161,10 @@ jobs: needs: - merge steps: + - name: Set IMAGE_NAME + run: | + echo "IMAGE_NAME=${GITHUB_REPOSITORY,,}" >>${GITHUB_ENV} + # Checkout code # https://github.com/actions/checkout - name: Checkout code