diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a0f0f80..f822e77 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,5 +1,6 @@ -name: Build and Test +name: Build Test and Lint on: + workflow_dispatch: pull_request: push: branches: @@ -7,9 +8,9 @@ on: - 'release-*.*' jobs: build: - name: Build and Lint + name: Build and Test runs-on: ubuntu-latest - timeout-minutes: 8 + timeout-minutes: 10 steps: # Checkout code # https://github.com/actions/checkout @@ -30,19 +31,13 @@ jobs: # Build Go binary - run: go build -v cmd/main.go - - name: Regenerate CRDs - run: make generate manifests - - - name: Check for CRD drift - run: | - git diff --compact-summary --exit-code || \ - (echo; echo "Unexpected difference in directories after code generation. Run 'make generate manifests' and commit."; exit 1) + - run: go test ./... - test: - name: Go Test + lint: + name: Lint needs: build runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 10 steps: # Checkout code # https://github.com/actions/checkout @@ -57,6 +52,23 @@ jobs: go-version-file: 'go.mod' cache: true - # Run Go tests - - name: Run go test - run: go test -v ./... \ No newline at end of file + - name: Install Helm + uses: azure/setup-helm@v3.5 + + # Run Go linters + # https://github.com/golangci/golangci-lint-action + - name: Run linters + uses: golangci/golangci-lint-action@v7 + with: + version: v2.4.0 + + - name: Regenerate CRDs + run: make generate manifests + + - name: Check for CRD drift + run: | + git diff --compact-summary --exit-code -- config/crd deploy/charts || \ + (echo; echo "Unexpected difference in directories after code generation. Run 'make generate manifests' and commit."; exit 1) + + - name: Lint Helm manifests + run: make lint-manifests \ No newline at end of file diff --git a/.golangci.yml b/.golangci.yml index 318dc6d..bd04587 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,32 +1,46 @@ -run: - # timeout for analysis, e.g. 30s, 5m, default is 1m - timeout: 12m - - skip-dirs: - - testdata$ - - test/mock - - go/pkg/mod - - skip-files: - - ".*\\.pb\\.go" - +version: "2" linters: enable: - bodyclose - durationcheck - errorlint - - goimports - - revive + - gocritic - gosec - misspell - nakedret + - nolintlint + - revive - unconvert - unparam - - whitespace - - gocritic - - nolintlint - -linters-settings: - revive: - # minimal confidence for issues, default is 0.8 - confidence: 0.0 + settings: + revive: + confidence: 0 + rules: + - name: var-naming + disabled: true + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + paths: + - .*\.pb\.go + - testdata$ + - test/mock + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - goimports + exclusions: + generated: lax + paths: + - .*\.pb\.go + - testdata$ + - test/mock + - third_party$ + - builtin$ + - examples$ \ No newline at end of file diff --git a/.kube-linter.yaml b/.kube-linter.yaml new file mode 100644 index 0000000..0b5b7ca --- /dev/null +++ b/.kube-linter.yaml @@ -0,0 +1,6 @@ +checks: + addAllBuiltIn: true + # Add project-specific exclusions below as needed, e.g.: + # exclude: + # - "unset-cpu-requirements" + # - "unset-memory-requirements" \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 551abd1..71e444c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# v2.5.2 +## Fixes +- Fixes an issue where a namespace may not be properly applied if applying the Helm template without a namespace specified / using `kubectl apply -f` directly with the rendered template. +- Fixes an issue where the error message from a failed Enrollment API call is not logged. +## Chores +- Update GitHub Actions workflow to check for policy enforcement on Helm chart rendered manifests in addition to checking for drift in generated CRDs. +- Fixes various linting issues in the codebase. + # v2.5.1 ## Fixes - Fixes an issue where OAuth 2.0 client credentials were being regenerated on every API call. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..2669087 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,81 @@ +# Command Cert Manager Issuer Contribution Guide + +## Requirements +- Go (>= 1.24) +- golangci-lint (>= 2.4.0) ([installation notes](https://github.com/golangci/golangci-lint?tab=readme-ov-file#install-golangci-lint)) +- helm (>= 3.x) — required to render chart templates for manifest linting ([installation notes](https://helm.sh/docs/intro/install/)) +- conftest — policy testing tool powered by Open Policy Agent; installed automatically by `make lint-manifests` + +## Installing dependencies +Project dependencies can be installed by running the following: + +```bash +go mod download +``` + +The following command can be used to add missing requirements or remove unused modules: + +```bash +go mod tidy +``` + +## Running unit tests +The following command can be run to run the project unit tests: + +```bash +go test -v ./... +``` + +## Running linters +The project uses golangci-lint to lint the codebase. The following command can be run to run the linters: + +```bash +golangci-lint run +``` + +or, alternatively: + +```bash +make lint +``` + +## Updating generated manifests + +This command will update the generated custom resource definitions under `config/crd/bases`: + +```bash +make generate manifests +``` + +> [!IMPORTANT] +> There is no automated process to automatically update the CRDs under `deploy/charts/command-cert-manager-issuer`. If any changes are made to the CRDs, the generated CRDs under `config/crd/bases` must be copied to `deploy/charts/command-cert-manager-issuer/crds` to ensure the Helm chart is up to date. + +## Linting Helm manifests + +The Helm chart under `deploy/charts/command-cert-manager-issuer` is linted with two tools on every PR: +- **conftest** — runs custom Rego policies located in the [`policy/`](policy/) directory against the rendered manifests + +To run both checks locally: + +```bash +make lint-manifests +``` + +`conftest` is downloaded automatically into `bin/` on first use; no manual installation is required. + +To inspect the rendered templates without linting: + +```bash +make helm-template +``` + +### Adding or modifying policies + +Rego policies live in [`policy/`](policy/). Each `.rego` file in that directory is evaluated by conftest against every resource in the rendered chart. Add a new `.rego` file to enforce additional rules. For example, `policy/roles.rego` enforces that all `Role` resources declare an explicit namespace. + +kube-linter checks can be tuned in [.kube-linter.yaml](.kube-linter.yaml). To exclude a check, add its name under the `exclude` key. + +## Running end-to-end tests +A comprehensive end-to-end test suite is available to verify the issuer code works against cert-manager and a Keyfactor Command instance. + +Instructions on how to run the end-to-end test suite can be found [here](./e2e/README.md). \ No newline at end of file diff --git a/Makefile b/Makefile index 7893a54..5a3b40d 100644 --- a/Makefile +++ b/Makefile @@ -17,6 +17,11 @@ endif # tools. (i.e. podman) CONTAINER_TOOL ?= docker +# Helm chart and Conftest policy directory for manifest linting +HELM_CHART_DIR ?= deploy/charts/command-cert-manager-issuer +HELM_RELEASE_NAME ?= command-cert-manager-issuer +POLICY_DIR ?= policy + # Setting SHELL to bash allows bash commands to be executed by recipes. # Options are set to exit when a recipe line exits non-zero or a piped command fails. SHELL = /usr/bin/env bash -o pipefail @@ -78,6 +83,20 @@ lint: golangci-lint ## Run golangci-lint linter & yamllint lint-fix: golangci-lint ## Run golangci-lint linter and perform fixes $(GOLANGCI_LINT) run --fix +.PHONY: helm-template +helm-template: ## Render Helm chart templates to stdout (includes CRDs). + helm template $(HELM_RELEASE_NAME) $(HELM_CHART_DIR) --include-crds + +.PHONY: lint-manifests +lint-manifests: conftest ## Run Conftest policy checks against every CI values file in $(HELM_CHART_DIR)/ci/. + @failed=0; \ + for f in $(HELM_CHART_DIR)/ci/*-values.yaml; do \ + echo "==> $$(basename $$f)"; \ + helm template $(HELM_RELEASE_NAME) $(HELM_CHART_DIR) --include-crds -f "$$f" \ + | $(CONFTEST) test --policy $(POLICY_DIR) - || failed=1; \ + done; \ + exit $$failed + ##@ Build .PHONY: build @@ -162,18 +181,32 @@ KUSTOMIZE ?= $(LOCALBIN)/kustomize-$(KUSTOMIZE_VERSION) CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen-$(CONTROLLER_TOOLS_VERSION) ENVTEST ?= $(LOCALBIN)/setup-envtest-$(ENVTEST_VERSION) GOLANGCI_LINT = $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION) +KUBE_LINTER = $(LOCALBIN)/kube-linter-$(KUBE_LINTER_VERSION) +CONFTEST = $(LOCALBIN)/conftest-$(CONFTEST_VERSION) ## Tool Versions KUSTOMIZE_VERSION ?= v5.3.0 CONTROLLER_TOOLS_VERSION ?= v0.14.0 ENVTEST_VERSION ?= latest -GOLANGCI_LINT_VERSION ?= v1.60.1 +GOLANGCI_LINT_VERSION ?= v2.4.0 +KUBE_LINTER_VERSION ?= v0.6.8 +CONFTEST_VERSION ?= v0.60.0 .PHONY: kustomize kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. $(KUSTOMIZE): $(LOCALBIN) $(call go-install-tool,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v5,$(KUSTOMIZE_VERSION)) +.PHONY: kube-linter +kube-linter: $(KUBE_LINTER) ## Download kube-linter locally if necessary. +$(KUBE_LINTER): $(LOCALBIN) + $(call go-install-tool,$(KUBE_LINTER),golang.stackrox.io/kube-linter/cmd/kube-linter,$(KUBE_LINTER_VERSION)) + +.PHONY: conftest +conftest: $(CONFTEST) ## Download conftest locally if necessary. +$(CONFTEST): $(LOCALBIN) + $(call go-install-tool,$(CONFTEST),github.com/open-policy-agent/conftest,$(CONFTEST_VERSION)) + .PHONY: controller-gen controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessary. $(CONTROLLER_GEN): $(LOCALBIN) @@ -187,7 +220,12 @@ $(ENVTEST): $(LOCALBIN) .PHONY: golangci-lint golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. $(GOLANGCI_LINT): $(LOCALBIN) - $(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint,${GOLANGCI_LINT_VERSION}) + @[ -f $(GOLANGCI_LINT) ] || { \ + set -e; \ + echo "Downloading golangci-lint $(GOLANGCI_LINT_VERSION)" ;\ + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/$(GOLANGCI_LINT_VERSION)/install.sh | sh -s -- -b $(LOCALBIN) $(GOLANGCI_LINT_VERSION) ;\ + mv $(LOCALBIN)/golangci-lint $(GOLANGCI_LINT) ;\ + } # go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist # $1 - target path with name of binary (ideally with version) diff --git a/api/v1alpha1/issuer_types.go b/api/v1alpha1/issuer_types.go index 2b5b669..deba1d6 100644 --- a/api/v1alpha1/issuer_types.go +++ b/api/v1alpha1/issuer_types.go @@ -201,10 +201,9 @@ func (is *IssuerStatus) HasCondition(conditionType IssuerConditionType, state Co } func (is *IssuerStatus) UnsetCondition(conditionType IssuerConditionType) { - conditions := is.Conditions - for i, c := range conditions { + for i, c := range is.Conditions { if c.Type == conditionType { - is.Conditions = append(conditions[:i], conditions[i+1:]...) + is.Conditions = append(is.Conditions[:i], is.Conditions[i+1:]...) return } } diff --git a/api/v1alpha1/issuer_types_test.go b/api/v1alpha1/issuer_types_test.go index 261df05..0628398 100644 --- a/api/v1alpha1/issuer_types_test.go +++ b/api/v1alpha1/issuer_types_test.go @@ -68,7 +68,7 @@ func TestIssuerStatus_SetCondition_UpdateConditionStatus(t *testing.T) { assert.Equal(t, "NewMessage", cond.Message) // LastTransitionTime should be updated because status changed from ConditionFalse -> ConditionTrue - assert.True(t, cond.LastTransitionTime.Time.After(now.Time), "LastTransitionTime should be more recent if the status changed.") + assert.True(t, cond.LastTransitionTime.After(now.Time), "LastTransitionTime should be more recent if the status changed.") } func TestIssuerStatus_SetCondition_NoStatusChange(t *testing.T) { diff --git a/cmd/main.go b/cmd/main.go index 387b682..124a8f4 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -197,7 +197,7 @@ func main() { } if defaultHealthCheckInterval < time.Duration(30)*time.Second { - setupLog.Error(errors.New(fmt.Sprintf("interval %s is invalid, must be greater than or equal to '30s'", healthCheckInterval)), "invalid health check interval") + setupLog.Error(fmt.Errorf("interval %s is invalid, must be greater than or equal to '30s'", healthCheckInterval), "invalid health check interval") os.Exit(1) } diff --git a/deploy/charts/command-cert-manager-issuer/ci/cluster-access-values.yaml b/deploy/charts/command-cert-manager-issuer/ci/cluster-access-values.yaml new file mode 100644 index 0000000..9cb9785 --- /dev/null +++ b/deploy/charts/command-cert-manager-issuer/ci/cluster-access-values.yaml @@ -0,0 +1,7 @@ +# Cluster-wide access configuration — exercises ClusterRole/ClusterRoleBinding +# paths for secret and configmap access, and enables secure metrics. +secretConfig: + useClusterRoleForSecretAccess: true + useClusterRoleForConfigMapAccess: true +metrics: + secure: true \ No newline at end of file diff --git a/deploy/charts/command-cert-manager-issuer/ci/default-values.yaml b/deploy/charts/command-cert-manager-issuer/ci/default-values.yaml new file mode 100644 index 0000000..16d98a7 --- /dev/null +++ b/deploy/charts/command-cert-manager-issuer/ci/default-values.yaml @@ -0,0 +1,7 @@ +# Default configuration — all flags at their chart defaults. +# Role/RoleBinding namespace-scoped paths are exercised. +secretConfig: + useClusterRoleForSecretAccess: false + useClusterRoleForConfigMapAccess: false +metrics: + secure: false \ No newline at end of file diff --git a/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml b/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml index 13fd364..f786533 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml @@ -42,6 +42,7 @@ rules: - apiGroups: - command-issuer.keyfactor.com resources: + - clusterissuers/finalizers - issuers/finalizers verbs: - update diff --git a/deploy/charts/command-cert-manager-issuer/templates/role.yaml b/deploy/charts/command-cert-manager-issuer/templates/role.yaml index eff6781..00acf3d 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/role.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/role.yaml @@ -4,6 +4,7 @@ metadata: labels: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} name: {{ include "command-cert-manager-issuer.name" . }}-leader-election-role + namespace: {{ .Release.Namespace }} rules: - apiGroups: - coordination.k8s.io @@ -31,6 +32,9 @@ metadata: labels: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} name: {{ include "command-cert-manager-issuer.name" . }}-secret-reader-role + {{- if not .Values.secretConfig.useClusterRoleForSecretAccess }} + namespace: {{ .Release.Namespace }} + {{- end }} rules: - apiGroups: - "" @@ -47,6 +51,9 @@ metadata: labels: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} name: {{ include "command-cert-manager-issuer.name" . }}-configmap-reader-role + {{- if not .Values.secretConfig.useClusterRoleForConfigMapAccess }} + namespace: {{ .Release.Namespace }} + {{- end }} rules: - apiGroups: - "" diff --git a/deploy/charts/command-cert-manager-issuer/templates/rolebinding.yaml b/deploy/charts/command-cert-manager-issuer/templates/rolebinding.yaml index 1125fd9..9ed93ec 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/rolebinding.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/rolebinding.yaml @@ -4,6 +4,7 @@ metadata: labels: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} name: {{ include "command-cert-manager-issuer.name" . }}-leader-election-rolebinding + namespace: {{ .Release.Namespace }} roleRef: apiGroup: rbac.authorization.k8s.io kind: Role @@ -19,6 +20,9 @@ metadata: labels: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} name: {{ include "command-cert-manager-issuer.name" . }}-secret-reader-rolebinding + {{- if not .Values.secretConfig.useClusterRoleForSecretAccess }} + namespace: {{ .Release.Namespace }} + {{- end }} roleRef: apiGroup: rbac.authorization.k8s.io kind: {{ if .Values.secretConfig.useClusterRoleForSecretAccess }}ClusterRole{{ else }}Role{{ end }} @@ -34,6 +38,9 @@ metadata: labels: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} name: {{ include "command-cert-manager-issuer.name" . }}-configmap-reader-rolebinding + {{- if not .Values.secretConfig.useClusterRoleForConfigMapAccess }} + namespace: {{ .Release.Namespace }} + {{- end }} roleRef: apiGroup: rbac.authorization.k8s.io kind: {{ if .Values.secretConfig.useClusterRoleForConfigMapAccess }}ClusterRole{{ else }}Role{{ end }} diff --git a/deploy/charts/command-cert-manager-issuer/templates/serviceaccount.yaml b/deploy/charts/command-cert-manager-issuer/templates/serviceaccount.yaml index ccefb2e..948bf2a 100644 --- a/deploy/charts/command-cert-manager-issuer/templates/serviceaccount.yaml +++ b/deploy/charts/command-cert-manager-issuer/templates/serviceaccount.yaml @@ -3,6 +3,7 @@ apiVersion: v1 kind: ServiceAccount metadata: name: {{ include "command-cert-manager-issuer.serviceAccountName" . }} + namespace: {{ .Release.Namespace }} labels: {{- include "command-cert-manager-issuer.labels" . | nindent 4 }} {{- if .Values.serviceAccount.labels }} diff --git a/internal/command/client.go b/internal/command/client.go index 36616e3..416ebb7 100644 --- a/internal/command/client.go +++ b/internal/command/client.go @@ -18,6 +18,7 @@ package command import ( "fmt" + "io" "net/http" "strings" "time" @@ -49,7 +50,7 @@ func setAmbientTokenCredentialSource(source TokenCredentialSource) { type Client interface { EnrollCSR(v1.ApiCreateEnrollmentCSRRequest) (*v1.CSSCMSDataModelModelsEnrollmentCSREnrollmentResponse, *http.Response, error) - GetAllMetadataFields(v1.ApiGetMetadataFieldsRequest) ([]v1.CSSCMSDataModelModelsMetadataType, *http.Response, error) + GetAllMetadataFields(v1.ApiGetMetadataFieldsRequest) ([]v1.CSSCMSDataModelModelsMetadataType, error) GetEnrollmentPatterns(v1.ApiGetEnrollmentPatternsRequest) ([]v1.EnrollmentPatternsEnrollmentPatternResponse, *http.Response, error) TestConnection() error } @@ -65,16 +66,21 @@ type clientAdapter struct { testConnection func() error } +// GetAllMetadataFields implements Client. Closes the response body internally so callers don't need to. +func (c *clientAdapter) GetAllMetadataFields(r v1.ApiGetMetadataFieldsRequest) ([]v1.CSSCMSDataModelModelsMetadataType, error) { + fields, resp, err := c.getAllMetadataFields(r) + if resp != nil && resp.Body != nil { + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + } + return fields, err +} + // EnrollCSR implements CertificateClient. func (c *clientAdapter) EnrollCSR(r v1.ApiCreateEnrollmentCSRRequest) (*v1.CSSCMSDataModelModelsEnrollmentCSREnrollmentResponse, *http.Response, error) { return c.enrollCSR(r) } -// GetAllMetadataFields implements Client. -func (c *clientAdapter) GetAllMetadataFields(r v1.ApiGetMetadataFieldsRequest) ([]v1.CSSCMSDataModelModelsMetadataType, *http.Response, error) { - return c.getAllMetadataFields(r) -} - func (c *clientAdapter) GetEnrollmentPatterns(r v1.ApiGetEnrollmentPatternsRequest) ([]v1.EnrollmentPatternsEnrollmentPatternResponse, *http.Response, error) { return c.getEnrollmentPatterns(r) } @@ -189,7 +195,7 @@ func (g *gcp) GetAccessToken(ctx context.Context) (string, error) { if err != nil { return "", fmt.Errorf("%w: failed to find GCP ADC: %w", errTokenFetchFailure, err) } - log.Info(fmt.Sprintf("generating a Google OIDC ID token...")) + log.Info("generating a Google OIDC ID token...") // Default audience to "command" if not provided aud := getValueOrDefault(g.audience, "command") @@ -220,7 +226,9 @@ func (g *gcp) GetAccessToken(ctx context.Context) (string, error) { // Only want to output this once, don't want to output this every time the JWT is generated log.Info("==== BEGIN DEBUG: Default Google ID Token JWT ======") + printClaims(log, token.AccessToken, []string{"aud", "iss", "sub", "email"}) + log.Info("==== END DEBUG: Default Google ID Token JWT ======") } @@ -242,11 +250,11 @@ func newGCPDefaultCredentialSource(ctx context.Context, audience string, scopes return source, nil } -func printClaims(log logr.Logger, token string, claimsToPrint []string) error { +func printClaims(log logr.Logger, token string, claimsToPrint []string) { tokenRaw, _, err := new(jwt.Parser).ParseUnverified(token, jwt.MapClaims{}) if err != nil { log.Error(err, "failed to parse JWT") - return fmt.Errorf("failed to parse JWT: %w", err) + return } claims, _ := tokenRaw.Claims.(jwt.MapClaims) @@ -258,9 +266,7 @@ func printClaims(log logr.Logger, token string, claimsToPrint []string) error { } } - if issuer, err := claims.GetIssuer(); err != nil { + if issuer, err := claims.GetIssuer(); err == nil && issuer != "" { log.Info(fmt.Sprintf("\nNOTE: If you are receiving a HTTP 401 on requests to Command, make sure an identity provider in Command is configured with '%s' as the authority.\nThe discovery endpoint for your issuer can be found at %s/.well-known/openid-configuration.", issuer, issuer)) } - - return nil } diff --git a/internal/command/client_test.go b/internal/command/client_test.go index 80b3b78..845064d 100644 --- a/internal/command/client_test.go +++ b/internal/command/client_test.go @@ -21,7 +21,6 @@ import ( "github.com/go-logr/logr/testr" "github.com/golang-jwt/jwt/v5" - "github.com/stretchr/testify/assert" ) func TestPrintClaims(t *testing.T) { @@ -36,8 +35,7 @@ func TestPrintClaims(t *testing.T) { token := createUnsignedJWT(t, claims) // Call the function - err := printClaims(testLogger, token, []string{"aud", "iss", "sub"}) - assert.NoError(t, err) + printClaims(testLogger, token, []string{"aud", "iss", "sub"}) }) t.Run("jwt with no issuer does not error", func(t *testing.T) { @@ -49,8 +47,7 @@ func TestPrintClaims(t *testing.T) { token := createUnsignedJWT(t, claims) // Call the function - err := printClaims(testLogger, token, []string{"aud", "iss", "sub"}) - assert.NoError(t, err) + printClaims(testLogger, token, []string{"aud", "iss", "sub"}) }) t.Run("jwt with empty claims does not error", func(t *testing.T) { @@ -59,20 +56,17 @@ func TestPrintClaims(t *testing.T) { token := createUnsignedJWT(t, claims) // Call the function - err := printClaims(testLogger, token, []string{"aud", "iss", "sub"}) - assert.NoError(t, err) + printClaims(testLogger, token, []string{"aud", "iss", "sub"}) }) t.Run("invalid jwt returns an error", func(t *testing.T) { // Call the function - err := printClaims(testLogger, "abcdefghijklmnop", []string{"aud", "iss", "sub"}) - assert.Error(t, err) + printClaims(testLogger, "abcdefghijklmnop", []string{"aud", "iss", "sub"}) }) t.Run("jwt with empty payload returns error", func(t *testing.T) { // Call the function - err := printClaims(testLogger, "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0..", []string{"aud", "iss", "sub"}) - assert.Error(t, err) + printClaims(testLogger, "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0..", []string{"aud", "iss", "sub"}) }) } diff --git a/internal/command/command.go b/internal/command/command.go index 025175f..a88a3d5 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -320,7 +320,7 @@ func (s *signer) Check(ctx context.Context) error { // CommandSupportsMetadata implements HealthChecker. func (s *signer) CommandSupportsMetadata() (bool, error) { - existingFields, _, err := s.client.GetAllMetadataFields(v1.ApiGetMetadataFieldsRequest{}) + existingFields, err := s.client.GetAllMetadataFields(v1.ApiGetMetadataFieldsRequest{}) if err != nil { return false, fmt.Errorf("failed to fetch metadata fields from connected Command instance: %w", err) } @@ -373,7 +373,7 @@ func (s *signer) Sign(ctx context.Context, csrBytes []byte, config *SignConfig) return nil, nil, err } - request, model, caBuilder, err := s.buildCsrEnrollRequest(ctx, config, k8sLog, csrBytes) + request, model, caBuilder, err := s.buildCsrEnrollRequest(config, k8sLog, csrBytes) if err != nil { return nil, nil, err } @@ -409,7 +409,7 @@ func (s *signer) Sign(ctx context.Context, csrBytes []byte, config *SignConfig) } if certificateOwnerId != nil || certificateOwnerName != nil { - detail += fmt.Sprintf(". Make sure the cert manager issuer's security context is assigned to the owner role name or ID.") + detail += ". Make sure the cert manager issuer's security context is assigned to the owner role name or ID." } if len(extractMetadataFromAnnotations(config.Annotations)) > 0 { @@ -421,8 +421,10 @@ func (s *signer) Sign(ctx context.Context, csrBytes []byte, config *SignConfig) defer httpResp.Body.Close() bodyBytes, err := io.ReadAll(httpResp.Body) - if err != nil { - detail += fmt.Sprintf(". Error response: %s", string(bodyBytes)) + if err == nil { + detail += fmt.Sprintf(". Error response from EnrollCSR API call: %s", string(bodyBytes)) + } else { + k8sLog.Error(err, "failed to read response body from failed EnrollCSR API call") } } } @@ -468,7 +470,7 @@ func extractMetadataFromAnnotations(annotations map[string]string) map[string]in } // Builds the CSR Enrollment request we will send to the /Enroll/CSR endpoint -func (s *signer) buildCsrEnrollRequest(ctx context.Context, config *SignConfig, k8sLog logr.Logger, csrBytes []byte) (*v1.ApiCreateEnrollmentCSRRequest, *v1.EnrollmentCSREnrollmentRequest, *strings.Builder, error) { +func (s *signer) buildCsrEnrollRequest(config *SignConfig, k8sLog logr.Logger, csrBytes []byte) (*v1.ApiCreateEnrollmentCSRRequest, *v1.EnrollmentCSREnrollmentRequest, *strings.Builder, error) { // Override defaults from annotations if err := updateConfigWithOverrides(config, k8sLog); err != nil { return nil, nil, nil, err @@ -517,7 +519,7 @@ func (s *signer) buildCsrEnrollRequest(ctx context.Context, config *SignConfig, k8sLog.Info(fmt.Sprintf("Using enrollment pattern ID from config. ID: %d", config.EnrollmentPatternId)) enrollmentPatternId = &config.EnrollmentPatternId } else if config.EnrollmentPatternName != "" { - pattern, err := getEnrollmentPatternByName(ctx, k8sLog, s, config.EnrollmentPatternName) + pattern, err := getEnrollmentPatternByName(k8sLog, s, config.EnrollmentPatternName) if err != nil { return nil, nil, nil, err } @@ -613,7 +615,7 @@ func getMetadataOverrideOrCurrentValue[T any](currentValue T, annotations map[st var conv int64 conv, err = strconv.ParseInt(value, 10, 32) if err != nil { - return zero, fmt.Errorf("%w: failed to parse %s from annotations: %s", errInvalidSignerConfig, key, err) + return zero, fmt.Errorf("%w: failed to parse %s from annotations: %w", errInvalidSignerConfig, key, err) } result = int32(conv) case string: @@ -649,19 +651,21 @@ func ptr[T any](v T) *T { } // getEnrollmentPatternByName retrieves an enrollment pattern by its name from Command. -func getEnrollmentPatternByName(ctx context.Context, log logr.Logger, s *signer, enrollmentPatternName string) (*v1.EnrollmentPatternsEnrollmentPatternResponse, error) { +func getEnrollmentPatternByName(log logr.Logger, s *signer, enrollmentPatternName string) (*v1.EnrollmentPatternsEnrollmentPatternResponse, error) { log.Info(fmt.Sprintf("Looking up enrollment pattern %q in Command...", enrollmentPatternName)) var model *v1.EnrollmentPatternsEnrollmentPatternResponse queryString := fmt.Sprintf("Name -eq \"%s\"", enrollmentPatternName) patterns, httpResp, err := s.client.GetEnrollmentPatterns(v1.ApiGetEnrollmentPatternsRequest{}.QueryString(queryString)) + if httpResp != nil && httpResp.Body != nil { + defer httpResp.Body.Close() + } if err != nil { // Capture the error message which should indicate the failure reason msg := "" if httpResp != nil && httpResp.Body != nil { - defer httpResp.Body.Close() bodyBytes, _ := io.ReadAll(httpResp.Body) msg += string(bodyBytes) } diff --git a/internal/command/command_test.go b/internal/command/command_test.go index bd46235..ac0faf5 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -17,6 +17,7 @@ limitations under the License. package command import ( + "bytes" "context" "crypto/ecdsa" "crypto/elliptic" @@ -27,6 +28,7 @@ import ( "encoding/pem" "errors" "fmt" + "io" "math/big" "net" "net/http" @@ -260,26 +262,6 @@ func TestSignConfigValidate(t *testing.T) { } } -type fakeCommandAuthenticator struct { - client *http.Client - config *auth_providers.Server -} - -// Authenticate implements api.AuthConfig. -func (f *fakeCommandAuthenticator) Authenticate() error { - return nil -} - -// GetHttpClient implements api.AuthConfig. -func (f *fakeCommandAuthenticator) GetHttpClient() (*http.Client, error) { - return f.client, nil -} - -// GetServerConfig implements api.AuthConfig. -func (f *fakeCommandAuthenticator) GetServerConfig() *auth_providers.Server { - return f.config -} - func TestNewServerConfig(t *testing.T) { testCases := map[string]struct { @@ -366,8 +348,9 @@ var ( ) type fakeClient struct { - enrollCallback func(v1.ApiCreateEnrollmentCSRRequest) - enrollResponse *v1.CSSCMSDataModelModelsEnrollmentCSREnrollmentResponse + enrollCallback func(v1.ApiCreateEnrollmentCSRRequest) + enrollResponse *v1.CSSCMSDataModelModelsEnrollmentCSREnrollmentResponse + enrollHTTPResponse *http.Response metadataFields []v1.CSSCMSDataModelModelsMetadataType enrollmentPatterns []v1.EnrollmentPatternsEnrollmentPatternResponse @@ -380,12 +363,12 @@ func (f *fakeClient) EnrollCSR(r v1.ApiCreateEnrollmentCSRRequest) (*v1.CSSCMSDa if f.enrollCallback != nil { f.enrollCallback(r) } - return f.enrollResponse, nil, f.err + return f.enrollResponse, f.enrollHTTPResponse, f.err } // GetAllMetadataFields implements Client. -func (f *fakeClient) GetAllMetadataFields(v1.ApiGetMetadataFieldsRequest) ([]v1.CSSCMSDataModelModelsMetadataType, *http.Response, error) { - return f.metadataFields, nil, f.err +func (f *fakeClient) GetAllMetadataFields(v1.ApiGetMetadataFieldsRequest) ([]v1.CSSCMSDataModelModelsMetadataType, error) { + return f.metadataFields, f.err } // GetEnrollmentPatterns implements Client. @@ -427,13 +410,15 @@ func TestSign(t *testing.T) { testCases := map[string]struct { enrollCSRFunctionError error + enrollHTTPResponse *http.Response enrollmentPatterns []v1.EnrollmentPatternsEnrollmentPatternResponse // Request config *SignConfig // Expected - expectedSignError error + expectedSignError error + expectedErrorContent *string }{ "success-no-meta-certificate-template": { // Request @@ -444,8 +429,6 @@ func TestSign(t *testing.T) { Meta: nil, Annotations: nil, }, - - expectedSignError: nil, }, "success-no-meta-enrollment-pattern-id": { // Request @@ -456,8 +439,6 @@ func TestSign(t *testing.T) { Meta: nil, Annotations: nil, }, - - expectedSignError: nil, }, "success-no-meta-enrollment-pattern-name": { enrollmentPatterns: []v1.EnrollmentPatternsEnrollmentPatternResponse{ @@ -475,8 +456,6 @@ func TestSign(t *testing.T) { Meta: nil, Annotations: nil, }, - - expectedSignError: nil, }, "success-no-meta-enrollment-pattern-id-overwrites-pattern-name": { enrollmentPatterns: []v1.EnrollmentPatternsEnrollmentPatternResponse{}, // This would fail if enrollment pattern name was used @@ -489,8 +468,6 @@ func TestSign(t *testing.T) { Meta: nil, Annotations: nil, }, - - expectedSignError: nil, }, "success-annotation-config-override-pattern-id": { // Request @@ -507,8 +484,6 @@ func TestSign(t *testing.T) { "command-issuer.keyfactor.com/enrollmentPatternId": "12345", }, }, - - expectedSignError: nil, }, "success-annotation-config-override-pattern-name": { enrollmentPatterns: []v1.EnrollmentPatternsEnrollmentPatternResponse{ @@ -532,8 +507,6 @@ func TestSign(t *testing.T) { "command-issuer.keyfactor.com/enrollmentPatternName": "enrollment-pattern-override", }, }, - - expectedSignError: nil, }, "success-no-meta-owner-role-id": { // Request @@ -545,8 +518,6 @@ func TestSign(t *testing.T) { Meta: nil, Annotations: nil, }, - - expectedSignError: nil, }, "success-no-meta-owner-role-name": { // Request @@ -558,8 +529,6 @@ func TestSign(t *testing.T) { Meta: nil, Annotations: nil, }, - - expectedSignError: nil, }, "success-predefined-meta": { // Request @@ -579,8 +548,6 @@ func TestSign(t *testing.T) { }, Annotations: nil, }, - - expectedSignError: nil, }, "success-custom-meta": { // Request @@ -593,8 +560,6 @@ func TestSign(t *testing.T) { fmt.Sprintf("%s%s", commandMetadataAnnotationPrefix, "testMetadata"): "test", }, }, - - expectedSignError: nil, }, "enroll-csr-err": { enrollCSRFunctionError: errors.New("an error from Command"), @@ -607,7 +572,27 @@ func TestSign(t *testing.T) { Annotations: nil, }, - expectedSignError: errCommandEnrollmentFailure, + expectedSignError: errCommandEnrollmentFailure, + expectedErrorContent: ptr("an error from Command"), + }, + "enroll-csr-err-response-body-included-in-error": { + enrollCSRFunctionError: errors.New("an error from Command"), + enrollHTTPResponse: &http.Response{ + StatusCode: http.StatusBadRequest, + Header: http.Header{}, + Body: io.NopCloser(bytes.NewReader([]byte(`{"Message":"certificate template not found"}`))), + }, + // Request + config: &SignConfig{ + CertificateTemplate: certificateTemplateName, + CertificateAuthorityLogicalName: certificateAuthorityLogicalName, + CertificateAuthorityHostname: certificateAuthorityHostname, + Meta: nil, + Annotations: nil, + }, + + expectedSignError: errCommandEnrollmentFailure, + expectedErrorContent: ptr("certificate template not found"), }, "enroll-csr-err-enrollment-pattern-not-found": { enrollmentPatterns: []v1.EnrollmentPatternsEnrollmentPatternResponse{}, @@ -635,6 +620,7 @@ func TestSign(t *testing.T) { err: tc.enrollCSRFunctionError, enrollResponse: certificateRestResponseFromExpectedCerts(t, expectedLeafAndChain, []*x509.Certificate{caCert}), + enrollHTTPResponse: tc.enrollHTTPResponse, enrollmentPatterns: tc.enrollmentPatterns, enrollCallback: cb, } @@ -642,13 +628,17 @@ func TestSign(t *testing.T) { client: &client, } - csrBytes, err := generateCSR("CN=command.example.org", nil, nil, nil) + csrBytes, err := generateCSR("CN=command.example.org", []string{"dns.example.com"}, []string{}, []string{}) require.NoError(t, err) csrPem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrBytes.Raw}) leafAndCA, root, err := signer.Sign(context.Background(), csrPem, tc.config) if tc.expectedSignError != nil { assertErrorIs(t, tc.expectedSignError, err) + + if tc.expectedErrorContent != nil { + assert.Contains(t, err.Error(), *tc.expectedErrorContent, "error message should contain content from response body when enrollCSR returns an error") + } } else { assert.NoError(t, err) @@ -849,7 +839,6 @@ func TestGetMetadataOverrideOrCurrentValue(t *testing.T) { } func TestBuildCsrEnrollRequest_Success(t *testing.T) { - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -858,7 +847,7 @@ func TestBuildCsrEnrollRequest_Success(t *testing.T) { } // Create test CSR - csrBytes, err := generateCSR("CN=command.example.org", nil, nil, nil) + csrBytes, err := generateCSR("CN=command1.example.org", nil, nil, nil) require.NoError(t, err) csrPem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrBytes.Raw}) @@ -884,7 +873,7 @@ func TestBuildCsrEnrollRequest_Success(t *testing.T) { }, } - req, result, caBuilder, err := signer.buildCsrEnrollRequest(ctx, config, logger, csrPem) + req, result, caBuilder, err := signer.buildCsrEnrollRequest(config, logger, csrPem) assert.NoError(t, err) @@ -937,7 +926,6 @@ func TestBuildCsrEnrollRequest_WithOverrides_CertificateTemplate(t *testing.T) { }, } - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -952,7 +940,7 @@ func TestBuildCsrEnrollRequest_WithOverrides_CertificateTemplate(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - _, result, _, err := signer.buildCsrEnrollRequest(ctx, &tc.config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(&tc.config, logger, csrPem) assert.NoError(t, err) assert.Equal(t, *tc.expected.Template.Get(), *result.Template.Get()) @@ -987,7 +975,6 @@ func TestBuildCsrEnrollRequest_WithOverrides_CertificateAuthorityLogicalName(t * }, } - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1002,7 +989,7 @@ func TestBuildCsrEnrollRequest_WithOverrides_CertificateAuthorityLogicalName(t * for name, tc := range testCases { t.Run(name, func(t *testing.T) { - _, result, _, err := signer.buildCsrEnrollRequest(ctx, &tc.config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(&tc.config, logger, csrPem) assert.NoError(t, err) assert.Equal(t, *tc.expected.CertificateAuthority.Get(), *result.CertificateAuthority.Get()) @@ -1037,7 +1024,6 @@ func TestBuildCsrEnrollRequest_WithOverrides_CertificateAuthorityHostname(t *tes }, } - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1052,7 +1038,7 @@ func TestBuildCsrEnrollRequest_WithOverrides_CertificateAuthorityHostname(t *tes for name, tc := range testCases { t.Run(name, func(t *testing.T) { - _, result, _, err := signer.buildCsrEnrollRequest(ctx, &tc.config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(&tc.config, logger, csrPem) assert.NoError(t, err) assert.Equal(t, *tc.expected.CertificateAuthority.Get(), *result.CertificateAuthority.Get()) @@ -1087,7 +1073,6 @@ func TestBuildCsrEnrollRequest_WithOverrides_EnrollmentPatternId(t *testing.T) { }, } - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1102,7 +1087,7 @@ func TestBuildCsrEnrollRequest_WithOverrides_EnrollmentPatternId(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - _, result, _, err := signer.buildCsrEnrollRequest(ctx, &tc.config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(&tc.config, logger, csrPem) assert.NoError(t, err) assert.Equal(t, *tc.expected.EnrollmentPatternId.Get(), *result.EnrollmentPatternId.Get()) @@ -1137,7 +1122,6 @@ func TestBuildCsrEnrollRequest_WithOverrides_OwnerRoleId(t *testing.T) { }, } - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1152,7 +1136,7 @@ func TestBuildCsrEnrollRequest_WithOverrides_OwnerRoleId(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - _, result, _, err := signer.buildCsrEnrollRequest(ctx, &tc.config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(&tc.config, logger, csrPem) assert.NoError(t, err) assert.Equal(t, *tc.expected.OwnerRoleId.Get(), *result.OwnerRoleId.Get()) @@ -1187,7 +1171,6 @@ func TestBuildCsrEnrollRequest_WithOverrides_OwnerRoleName(t *testing.T) { }, } - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1202,7 +1185,7 @@ func TestBuildCsrEnrollRequest_WithOverrides_OwnerRoleName(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - _, result, _, err := signer.buildCsrEnrollRequest(ctx, &tc.config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(&tc.config, logger, csrPem) assert.NoError(t, err) assert.Equal(t, *tc.expected.OwnerRoleName.Get(), *result.OwnerRoleName.Get()) @@ -1211,7 +1194,6 @@ func TestBuildCsrEnrollRequest_WithOverrides_OwnerRoleName(t *testing.T) { } func TestBuildCsrEnrollRequest_NoCertificateTemplate_TemplatePropertyIsNil(t *testing.T) { - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1228,7 +1210,7 @@ func TestBuildCsrEnrollRequest_NoCertificateTemplate_TemplatePropertyIsNil(t *te CertificateTemplate: "", } - _, result, _, err := signer.buildCsrEnrollRequest(ctx, config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(config, logger, csrPem) assert.NoError(t, err) assert.NotNil(t, result) @@ -1236,7 +1218,6 @@ func TestBuildCsrEnrollRequest_NoCertificateTemplate_TemplatePropertyIsNil(t *te } func TestBuildCsrEnrollRequest_EnrollmentPatternNameAndEnrollmentPatternIdPopulated_UsesEnrollmentPatternIdValue(t *testing.T) { - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1254,7 +1235,7 @@ func TestBuildCsrEnrollRequest_EnrollmentPatternNameAndEnrollmentPatternIdPopula EnrollmentPatternName: "TestEnrollmentPatternId", } - _, result, _, err := signer.buildCsrEnrollRequest(ctx, config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(config, logger, csrPem) assert.NoError(t, err) assert.NotNil(t, result) @@ -1262,7 +1243,6 @@ func TestBuildCsrEnrollRequest_EnrollmentPatternNameAndEnrollmentPatternIdPopula } func TestBuildCsrEnrollRequest_EnrollmentPatternNameIsPopulated_GetsEnrollmentPatternFromClient(t *testing.T) { - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{ @@ -1285,7 +1265,7 @@ func TestBuildCsrEnrollRequest_EnrollmentPatternNameIsPopulated_GetsEnrollmentPa EnrollmentPatternName: "TestEnrollmentPatternId", } - _, result, _, err := signer.buildCsrEnrollRequest(ctx, config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(config, logger, csrPem) assert.NoError(t, err) assert.NotNil(t, result) @@ -1293,7 +1273,6 @@ func TestBuildCsrEnrollRequest_EnrollmentPatternNameIsPopulated_GetsEnrollmentPa } func TestBuildCsrEnrollRequest_OwnerRoleIdIsPopulated_UsesOwnerRoleIdValue(t *testing.T) { - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1310,7 +1289,7 @@ func TestBuildCsrEnrollRequest_OwnerRoleIdIsPopulated_UsesOwnerRoleIdValue(t *te OwnerRoleId: 123, } - _, result, _, err := signer.buildCsrEnrollRequest(ctx, config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(config, logger, csrPem) assert.NoError(t, err) assert.NotNil(t, result) @@ -1318,7 +1297,6 @@ func TestBuildCsrEnrollRequest_OwnerRoleIdIsPopulated_UsesOwnerRoleIdValue(t *te } func TestBuildCsrEnrollRequest_OwnerRoleNameIsPopulated_UsesOwnerRoleNameValue(t *testing.T) { - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1335,7 +1313,7 @@ func TestBuildCsrEnrollRequest_OwnerRoleNameIsPopulated_UsesOwnerRoleNameValue(t OwnerRoleName: "TestOwnerRole", } - _, result, _, err := signer.buildCsrEnrollRequest(ctx, config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(config, logger, csrPem) assert.NoError(t, err) assert.NotNil(t, result) @@ -1343,7 +1321,6 @@ func TestBuildCsrEnrollRequest_OwnerRoleNameIsPopulated_UsesOwnerRoleNameValue(t } func TestBuildCsrEnrollRequest_OwnerRoleIdAndNameArePopulated_UsesOwnerRoleIdValue(t *testing.T) { - ctx := context.Background() logger := logr.FromContextOrDiscard(context.Background()) client := fakeClient{} @@ -1361,7 +1338,7 @@ func TestBuildCsrEnrollRequest_OwnerRoleIdAndNameArePopulated_UsesOwnerRoleIdVal OwnerRoleName: "TestOwnerRole", } - _, result, _, err := signer.buildCsrEnrollRequest(ctx, config, logger, csrPem) + _, result, _, err := signer.buildCsrEnrollRequest(config, logger, csrPem) assert.NoError(t, err) assert.NotNil(t, result) diff --git a/internal/controller/certificaterequest_controller.go b/internal/controller/certificaterequest_controller.go index 66d6ec7..8df7e24 100644 --- a/internal/controller/certificaterequest_controller.go +++ b/internal/controller/certificaterequest_controller.go @@ -69,7 +69,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R var certificateRequest cmapi.CertificateRequest if err := r.Get(ctx, req.NamespacedName, &certificateRequest); err != nil { if err := client.IgnoreNotFound(err); err != nil { - return ctrl.Result{}, fmt.Errorf("unexpected get error: %v", err) + return ctrl.Result{}, fmt.Errorf("unexpected get error: %w", err) } log.Info("CertificateRequest not found. ignoring.") return ctrl.Result{}, nil @@ -159,7 +159,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R issuerGVK := commandissuer.GroupVersion.WithKind(certificateRequest.Spec.IssuerRef.Kind) issuerRO, err := r.Scheme.New(issuerGVK) if err != nil { - err = fmt.Errorf("%w: %v", errIssuerRef, err) + err = fmt.Errorf("%w: %w", errIssuerRef, err) log.Error(err, "Unrecognized kind. Ignoring.") setCertificateRequestReadyCondition(&certificateRequest, cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, err.Error()) return ctrl.Result{}, nil @@ -215,7 +215,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R commandSigner, err := r.SignerBuilder(ctx, config) if err != nil { - return ctrl.Result{}, fmt.Errorf("%w: %v", errSignerBuilder, err) + return ctrl.Result{}, fmt.Errorf("%w: %w", errSignerBuilder, err) } // Assign metadata @@ -254,6 +254,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R certificateRequest.Status.CA = chain setCertificateRequestReadyCondition(&certificateRequest, cmmeta.ConditionTrue, cmapi.CertificateRequestReasonIssued, "Signed") + log.Info(fmt.Sprintf("CertificateRequest %s/%s successfully signed", certificateRequest.Namespace, certificateRequest.Name)) return ctrl.Result{}, nil } diff --git a/internal/controller/certificaterequest_controller_test.go b/internal/controller/certificaterequest_controller_test.go index e01d633..30af28b 100644 --- a/internal/controller/certificaterequest_controller_test.go +++ b/internal/controller/certificaterequest_controller_test.go @@ -18,15 +18,8 @@ package controller import ( "context" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "crypto/x509" - "crypto/x509/pkix" "errors" - "math/big" "testing" - "time" commandissuerv1alpha1 "github.com/Keyfactor/command-cert-manager-issuer/api/v1alpha1" "github.com/Keyfactor/command-cert-manager-issuer/internal/command" @@ -806,39 +799,3 @@ func assertCertificateRequestHasReadyCondition(t *testing.T, status cmmeta.Condi assert.Contains(t, validReasons, reason, "unexpected condition reason") assert.Equal(t, reason, condition.Reason, "unexpected condition reason") } - -func issueTestCertificate(t *testing.T, cn string, parent *x509.Certificate, signingKey any) (*x509.Certificate, *ecdsa.PrivateKey) { - var err error - var key *ecdsa.PrivateKey - now := time.Now() - - key, err = ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - publicKey := &key.PublicKey - signerPrivateKey := key - if signingKey != nil { - signerPrivateKey = signingKey.(*ecdsa.PrivateKey) - } - - serial, _ := rand.Int(rand.Reader, big.NewInt(1337)) - certTemplate := &x509.Certificate{ - Subject: pkix.Name{CommonName: cn}, - SerialNumber: serial, - BasicConstraintsValid: true, - IsCA: true, - NotBefore: now, - NotAfter: now.Add(time.Hour * 24), - } - - if parent == nil { - parent = certTemplate - } - - certData, err := x509.CreateCertificate(rand.Reader, certTemplate, parent, publicKey, signerPrivateKey) - require.NoError(t, err) - - cert, err := x509.ParseCertificate(certData) - require.NoError(t, err) - - return cert, key -} diff --git a/internal/controller/issuer_controller.go b/internal/controller/issuer_controller.go index aabbb2a..8753771 100644 --- a/internal/controller/issuer_controller.go +++ b/internal/controller/issuer_controller.go @@ -83,7 +83,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res } if err := r.Get(ctx, req.NamespacedName, issuer); err != nil { if err := client.IgnoreNotFound(err); err != nil { - return ctrl.Result{}, fmt.Errorf("unexpected get error: %v", err) + return ctrl.Result{}, fmt.Errorf("unexpected get error: %w", err) } log.Info(fmt.Sprintf("%s not found. Ignoring.", issuer.GetObjectKind().GroupVersionKind().Kind)) return ctrl.Result{}, nil @@ -205,8 +205,8 @@ func commandConfigFromIssuer(ctx context.Context, c client.Client, issuer comman return nil, fmt.Errorf("%w, secret name: %s, reason: %w", errGetAuthSecret, issuer.GetSpec().SecretName, err) } - switch { - case authSecret.Type == corev1.SecretTypeOpaque: + switch authSecret.Type { + case corev1.SecretTypeOpaque: // We expect auth credentials for a client credential OAuth2.0 flow if the secret type is opaque tokenURL, ok := authSecret.Data[commandissuer.OAuthTokenURLKey] if !ok { @@ -235,7 +235,7 @@ func commandConfigFromIssuer(ctx context.Context, c client.Client, issuer comman } log.Info("Found oauth client credentials in secret", "commandSecretName", issuer.GetSpec().SecretName, "type", authSecret.Type) - case authSecret.Type == corev1.SecretTypeBasicAuth: + case corev1.SecretTypeBasicAuth: username, ok := authSecret.Data[corev1.BasicAuthUsernameKey] if !ok { return nil, fmt.Errorf("%w: %s", errGetAuthSecret, "found basic auth secret with no username") diff --git a/internal/controller/issuer_controller_test.go b/internal/controller/issuer_controller_test.go index 9e6764d..54fda8b 100644 --- a/internal/controller/issuer_controller_test.go +++ b/internal/controller/issuer_controller_test.go @@ -24,7 +24,6 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" commandissuer "github.com/Keyfactor/command-cert-manager-issuer/api/v1alpha1" - commandissuerv1alpha1 "github.com/Keyfactor/command-cert-manager-issuer/api/v1alpha1" "github.com/Keyfactor/command-cert-manager-issuer/internal/command" logrtesting "github.com/go-logr/logr/testing" "github.com/stretchr/testify/assert" @@ -71,8 +70,8 @@ func TestIssuerReconcile(t *testing.T) { defaultHealthCheckInterval *time.Duration expectedResult ctrl.Result expectedError error - expectedReadyConditionStatus commandissuerv1alpha1.ConditionStatus - expectedMetadataSupportedConditionStatus commandissuerv1alpha1.ConditionStatus + expectedReadyConditionStatus commandissuer.ConditionStatus + expectedMetadataSupportedConditionStatus commandissuer.ConditionStatus } tests := map[string]testCase{ @@ -80,23 +79,23 @@ func TestIssuerReconcile(t *testing.T) { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -114,31 +113,31 @@ func TestIssuerReconcile(t *testing.T) { }, }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "issuer-basicauth-no-username": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -155,30 +154,30 @@ func TestIssuerReconcile(t *testing.T) { }, }, expectedError: errGetAuthSecret, - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, }, "issuer-basicauth-no-password": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -195,29 +194,29 @@ func TestIssuerReconcile(t *testing.T) { }, }, expectedError: errGetAuthSecret, - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, }, "success-clusterissuer-basicauth": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.ClusterIssuer{ + &commandissuer.ClusterIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: "clusterissuer1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "clusterissuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -236,27 +235,27 @@ func TestIssuerReconcile(t *testing.T) { }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), clusterResourceNamespace: "kube-system", - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "success-issuer-oauth": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -268,40 +267,40 @@ func TestIssuerReconcile(t *testing.T) { Namespace: "ns1", }, Data: map[string][]byte{ - commandissuerv1alpha1.OAuthTokenURLKey: []byte("https://dev.idp.com/oauth/token"), - commandissuerv1alpha1.OAuthClientIDKey: []byte("fi3ElQUVoBBHyRNt4mpUxG9WY65AOCcJ"), - commandissuerv1alpha1.OAuthClientSecretKey: []byte("1EXHdD7Ikmmv0OkBoJZZtzOG5iAzvwdqBVuvquf-QEvL6fLrEG_heJHphtEXVj9H"), - commandissuerv1alpha1.OAuthScopesKey: []byte("read:certificates,write:certificates"), - commandissuerv1alpha1.OAuthAudienceKey: []byte("https://command.example.com"), + commandissuer.OAuthTokenURLKey: []byte("https://dev.idp.com/oauth/token"), + commandissuer.OAuthClientIDKey: []byte("fi3ElQUVoBBHyRNt4mpUxG9WY65AOCcJ"), + commandissuer.OAuthClientSecretKey: []byte("1EXHdD7Ikmmv0OkBoJZZtzOG5iAzvwdqBVuvquf-QEvL6fLrEG_heJHphtEXVj9H"), + commandissuer.OAuthScopesKey: []byte("read:certificates,write:certificates"), + commandissuer.OAuthAudienceKey: []byte("https://command.example.com"), }, }, }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "issuer-oauth-no-tokenurl": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -313,38 +312,38 @@ func TestIssuerReconcile(t *testing.T) { Namespace: "ns1", }, Data: map[string][]byte{ - commandissuerv1alpha1.OAuthClientIDKey: []byte("fi3ElQUVoBBHyRNt4mpUxG9WY65AOCcJ"), - commandissuerv1alpha1.OAuthClientSecretKey: []byte("1EXHdD7Ikmmv0OkBoJZZtzOG5iAzvwdqBVuvquf-QEvL6fLrEG_heJHphtEXVj9H"), - commandissuerv1alpha1.OAuthScopesKey: []byte("read:certificates,write:certificates"), - commandissuerv1alpha1.OAuthAudienceKey: []byte("https://command.example.com"), + commandissuer.OAuthClientIDKey: []byte("fi3ElQUVoBBHyRNt4mpUxG9WY65AOCcJ"), + commandissuer.OAuthClientSecretKey: []byte("1EXHdD7Ikmmv0OkBoJZZtzOG5iAzvwdqBVuvquf-QEvL6fLrEG_heJHphtEXVj9H"), + commandissuer.OAuthScopesKey: []byte("read:certificates,write:certificates"), + commandissuer.OAuthAudienceKey: []byte("https://command.example.com"), }, }, }, expectedError: errGetAuthSecret, - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, }, "issuer-oauth-no-clientid": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -356,38 +355,38 @@ func TestIssuerReconcile(t *testing.T) { Namespace: "ns1", }, Data: map[string][]byte{ - commandissuerv1alpha1.OAuthTokenURLKey: []byte("https://dev.idp.com/oauth/token"), - commandissuerv1alpha1.OAuthClientSecretKey: []byte("1EXHdD7Ikmmv0OkBoJZZtzOG5iAzvwdqBVuvquf-QEvL6fLrEG_heJHphtEXVj9H"), - commandissuerv1alpha1.OAuthScopesKey: []byte("read:certificates,write:certificates"), - commandissuerv1alpha1.OAuthAudienceKey: []byte("https://command.example.com"), + commandissuer.OAuthTokenURLKey: []byte("https://dev.idp.com/oauth/token"), + commandissuer.OAuthClientSecretKey: []byte("1EXHdD7Ikmmv0OkBoJZZtzOG5iAzvwdqBVuvquf-QEvL6fLrEG_heJHphtEXVj9H"), + commandissuer.OAuthScopesKey: []byte("read:certificates,write:certificates"), + commandissuer.OAuthAudienceKey: []byte("https://command.example.com"), }, }, }, expectedError: errGetAuthSecret, - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, }, "issuer-oauth-no-clientsecret": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -399,33 +398,33 @@ func TestIssuerReconcile(t *testing.T) { Namespace: "ns1", }, Data: map[string][]byte{ - commandissuerv1alpha1.OAuthTokenURLKey: []byte("https://dev.idp.com/oauth/token"), - commandissuerv1alpha1.OAuthClientIDKey: []byte("fi3ElQUVoBBHyRNt4mpUxG9WY65AOCcJ"), - commandissuerv1alpha1.OAuthScopesKey: []byte("read:certificates,write:certificates"), - commandissuerv1alpha1.OAuthAudienceKey: []byte("https://command.example.com"), + commandissuer.OAuthTokenURLKey: []byte("https://dev.idp.com/oauth/token"), + commandissuer.OAuthClientIDKey: []byte("fi3ElQUVoBBHyRNt4mpUxG9WY65AOCcJ"), + commandissuer.OAuthScopesKey: []byte("read:certificates,write:certificates"), + commandissuer.OAuthAudienceKey: []byte("https://command.example.com"), }, }, }, expectedError: errGetAuthSecret, - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, }, "success-clusterissuer-oauth": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.ClusterIssuer{ + &commandissuer.ClusterIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: "clusterissuer1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "clusterissuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -437,18 +436,18 @@ func TestIssuerReconcile(t *testing.T) { Namespace: "kube-system", }, Data: map[string][]byte{ - commandissuerv1alpha1.OAuthTokenURLKey: []byte("https://dev.idp.com/oauth/token"), - commandissuerv1alpha1.OAuthClientIDKey: []byte("fi3ElQUVoBBHyRNt4mpUxG9WY65AOCcJ"), - commandissuerv1alpha1.OAuthClientSecretKey: []byte("1EXHdD7Ikmmv0OkBoJZZtzOG5iAzvwdqBVuvquf-QEvL6fLrEG_heJHphtEXVj9H"), - commandissuerv1alpha1.OAuthScopesKey: []byte("read:certificates,write:certificates"), - commandissuerv1alpha1.OAuthAudienceKey: []byte("https://command.example.com"), + commandissuer.OAuthTokenURLKey: []byte("https://dev.idp.com/oauth/token"), + commandissuer.OAuthClientIDKey: []byte("fi3ElQUVoBBHyRNt4mpUxG9WY65AOCcJ"), + commandissuer.OAuthClientSecretKey: []byte("1EXHdD7Ikmmv0OkBoJZZtzOG5iAzvwdqBVuvquf-QEvL6fLrEG_heJHphtEXVj9H"), + commandissuer.OAuthScopesKey: []byte("read:certificates,write:certificates"), + commandissuer.OAuthAudienceKey: []byte("https://command.example.com"), }, }, }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), clusterResourceNamespace: "kube-system", - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "issuer-kind-Unrecognized": { @@ -461,52 +460,52 @@ func TestIssuerReconcile(t *testing.T) { "issuer-missing-secret": { name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, }, }, expectedError: errGetAuthSecret, - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, }, "issuer-failing-healthchecker-builder": { name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -526,29 +525,29 @@ func TestIssuerReconcile(t *testing.T) { healthCheckerBuilder: newFakeHealthCheckerBuilder(errors.New("simulated health checker builder error"), nil, false), expectedError: errHealthCheckerBuilder, - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, }, "issuer-failing-healthchecker-check": { name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, { - Type: commandissuerv1alpha1.IssuerConditionSupportsMetadata, - Status: commandissuerv1alpha1.ConditionFalse, + Type: commandissuer.IssuerConditionSupportsMetadata, + Status: commandissuer.ConditionFalse, }, }, }, @@ -567,30 +566,30 @@ func TestIssuerReconcile(t *testing.T) { }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, errors.New("simulated health check error"), false), expectedError: errHealthCheckerCheck, - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionFalse, }, "success-custom-healthcheck-interval-issuer": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", - HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + HealthCheck: &commandissuer.HealthCheckConfig{ Enabled: true, Interval: to.Ptr(metav1.Duration{Duration: 30 * time.Second}), }, }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -608,30 +607,30 @@ func TestIssuerReconcile(t *testing.T) { }, }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Duration(30) * time.Second}, }, "success-custom-healthcheck-interval-clusterissuer": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.ClusterIssuer{ + &commandissuer.ClusterIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: "clusterissuer1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "clusterissuer1-credentials", - HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + HealthCheck: &commandissuer.HealthCheckConfig{ Enabled: true, Interval: to.Ptr(metav1.Duration{Duration: 120 * time.Second}), }, }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -650,29 +649,29 @@ func TestIssuerReconcile(t *testing.T) { }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), clusterResourceNamespace: "kube-system", - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Duration(120) * time.Second}, }, "success-healthcheck-disabled": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.ClusterIssuer{ + &commandissuer.ClusterIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: "clusterissuer1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "clusterissuer1-credentials", - HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + HealthCheck: &commandissuer.HealthCheckConfig{ Enabled: false, }, }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -691,30 +690,30 @@ func TestIssuerReconcile(t *testing.T) { }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), clusterResourceNamespace: "kube-system", - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Duration(0)}, }, "success-no-healthcheck-interval": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.ClusterIssuer{ + &commandissuer.ClusterIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: "clusterissuer1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "clusterissuer1-credentials", - HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + HealthCheck: &commandissuer.HealthCheckConfig{ Enabled: true, Interval: nil, }, }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -733,30 +732,30 @@ func TestIssuerReconcile(t *testing.T) { }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), clusterResourceNamespace: "kube-system", - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Minute}, }, "success-nil-healthcheck-interval-defaults": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.ClusterIssuer{ + &commandissuer.ClusterIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: "clusterissuer1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "clusterissuer1-credentials", - HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + HealthCheck: &commandissuer.HealthCheckConfig{ Enabled: true, Interval: nil, }, }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -775,27 +774,27 @@ func TestIssuerReconcile(t *testing.T) { }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), clusterResourceNamespace: "kube-system", - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Duration(60) * time.Second}, }, "success-default-healthcheck-interval": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.ClusterIssuer{ + &commandissuer.ClusterIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: "clusterissuer1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "clusterissuer1-credentials", HealthCheck: nil, }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -815,27 +814,27 @@ func TestIssuerReconcile(t *testing.T) { defaultHealthCheckInterval: to.Ptr(time.Duration(2) * time.Minute), healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), clusterResourceNamespace: "kube-system", - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Duration(2) * time.Minute}, }, "success-nil-healthcheck-defaults": { kind: "ClusterIssuer", name: types.NamespacedName{Name: "clusterissuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.ClusterIssuer{ + &commandissuer.ClusterIssuer{ ObjectMeta: metav1.ObjectMeta{ Name: "clusterissuer1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "clusterissuer1-credentials", HealthCheck: nil, }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -854,31 +853,31 @@ func TestIssuerReconcile(t *testing.T) { }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true), clusterResourceNamespace: "kube-system", - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue, + expectedReadyConditionStatus: commandissuer.ConditionTrue, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionTrue, expectedResult: ctrl.Result{RequeueAfter: time.Duration(60) * time.Second}, }, "error-healthcheck-minimum-value": { kind: "Issuer", name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"}, objects: []client.Object{ - &commandissuerv1alpha1.Issuer{ + &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "issuer1", Namespace: "ns1", }, - Spec: commandissuerv1alpha1.IssuerSpec{ + Spec: commandissuer.IssuerSpec{ SecretName: "issuer1-credentials", - HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{ + HealthCheck: &commandissuer.HealthCheckConfig{ Enabled: true, Interval: to.Ptr(metav1.Duration{Duration: 29 * time.Second}), }, }, - Status: commandissuerv1alpha1.IssuerStatus{ - Conditions: []commandissuerv1alpha1.IssuerCondition{ + Status: commandissuer.IssuerStatus{ + Conditions: []commandissuer.IssuerCondition{ { - Type: commandissuerv1alpha1.IssuerConditionReady, - Status: commandissuerv1alpha1.ConditionUnknown, + Type: commandissuer.IssuerConditionReady, + Status: commandissuer.ConditionUnknown, }, }, }, @@ -896,14 +895,14 @@ func TestIssuerReconcile(t *testing.T) { }, }, healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false), - expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse, - expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionUnknown, + expectedReadyConditionStatus: commandissuer.ConditionFalse, + expectedMetadataSupportedConditionStatus: commandissuer.ConditionUnknown, expectedResult: ctrl.Result{}, }, } scheme := runtime.NewScheme() - require.NoError(t, commandissuerv1alpha1.AddToScheme(scheme)) + require.NoError(t, commandissuer.AddToScheme(scheme)) require.NoError(t, corev1.AddToScheme(scheme)) for name, tc := range tests { @@ -950,8 +949,8 @@ func TestIssuerReconcile(t *testing.T) { require.NoError(t, err) require.NoError(t, fakeClient.Get(context.TODO(), tc.name, issuer)) require.NoError(t, err) - assert.True(t, issuer.GetStatus().HasCondition(commandissuerv1alpha1.IssuerConditionReady, tc.expectedReadyConditionStatus)) - assert.True(t, issuer.GetStatus().HasCondition(commandissuerv1alpha1.IssuerConditionSupportsMetadata, tc.expectedMetadataSupportedConditionStatus)) + assert.True(t, issuer.GetStatus().HasCondition(commandissuer.IssuerConditionReady, tc.expectedReadyConditionStatus)) + assert.True(t, issuer.GetStatus().HasCondition(commandissuer.IssuerConditionSupportsMetadata, tc.expectedMetadataSupportedConditionStatus)) } }) } @@ -960,7 +959,7 @@ func TestIssuerReconcile(t *testing.T) { func TestCommandConfigFromIssuer(t *testing.T) { type testCase struct { name string - issuerSpec commandissuerv1alpha1.IssuerSpec + issuerSpec commandissuer.IssuerSpec secretNamespace string objects []client.Object expectedConfig *command.Config @@ -971,7 +970,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { tests := []testCase{ { name: "success-basic-auth", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", SecretName: "auth-secret", @@ -1003,7 +1002,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-basic-auth-with-ca-cert-secret", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", SecretName: "auth-secret", @@ -1047,7 +1046,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-basic-auth-with-ca-cert-secret-with-key", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", SecretName: "auth-secret", @@ -1093,7 +1092,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-basic-auth-with-ca-cert-configmap", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", SecretName: "auth-secret", @@ -1136,7 +1135,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-basic-auth-with-ca-cert-configmap-with-key", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", SecretName: "auth-secret", @@ -1181,7 +1180,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-basic-auth-with-ca-cert-configmap-overwrites-secret", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", SecretName: "auth-secret", @@ -1235,7 +1234,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-oauth-minimal", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", SecretName: "oauth-secret", @@ -1269,7 +1268,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-oauth-with-scopes-and-audience", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", SecretName: "oauth-secret", @@ -1307,7 +1306,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-ambient-credentials-with-scopes", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", Scopes: "scope1,scope2", @@ -1324,7 +1323,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-no-auth-secret", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", APIPath: "/api/v1", }, @@ -1339,7 +1338,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-auth-secret-not-found", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", SecretName: "missing-secret", }, @@ -1349,7 +1348,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-ca-secret-not-found", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", CaSecretName: "missing-ca-secret", }, @@ -1359,7 +1358,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-ca-secret-key-not-found", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", CaSecretName: "ca-secret", CaBundleKey: "ca.crt", @@ -1381,7 +1380,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-ca-configmap-not-found", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", CaBundleConfigMapName: "missing-ca-bundle", }, @@ -1391,7 +1390,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-ca-configmap-key-not-found", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", CaBundleConfigMapName: "ca-configmap", CaBundleKey: "ca.crt", @@ -1412,7 +1411,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-basic-auth-no-username", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", SecretName: "auth-secret", }, @@ -1434,7 +1433,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-basic-auth-no-password", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", SecretName: "auth-secret", }, @@ -1456,7 +1455,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-oauth-no-token-url", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", SecretName: "oauth-secret", }, @@ -1479,7 +1478,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-oauth-no-client-id", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", SecretName: "oauth-secret", }, @@ -1502,7 +1501,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-oauth-no-client-secret", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", SecretName: "oauth-secret", }, @@ -1525,7 +1524,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "error-unsupported-secret-type", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", SecretName: "auth-secret", }, @@ -1548,7 +1547,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { }, { name: "success-cluster-scoped-secret-namespace", - issuerSpec: commandissuerv1alpha1.IssuerSpec{ + issuerSpec: commandissuer.IssuerSpec{ Hostname: "https://ca.example.com", SecretName: "auth-secret", }, @@ -1579,7 +1578,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { } scheme := runtime.NewScheme() - require.NoError(t, commandissuerv1alpha1.AddToScheme(scheme)) + require.NoError(t, commandissuer.AddToScheme(scheme)) require.NoError(t, corev1.AddToScheme(scheme)) for _, tc := range tests { @@ -1590,7 +1589,7 @@ func TestCommandConfigFromIssuer(t *testing.T) { Build() // Create a minimal issuer with the test spec - issuer := &commandissuerv1alpha1.Issuer{ + issuer := &commandissuer.Issuer{ ObjectMeta: metav1.ObjectMeta{ Name: "test-issuer", Namespace: tc.secretNamespace, diff --git a/policy/deployments.rego b/policy/deployments.rego new file mode 100644 index 0000000..ab0e6cf --- /dev/null +++ b/policy/deployments.rego @@ -0,0 +1,31 @@ +package main + +import rego.v1 + +# Validate that every Deployment container and initContainer effectively runs +# as non-root, either by setting securityContext.runAsNonRoot=true itself or +# by inheriting a pod-level default. +pod_run_as_non_root_default if { + object.get(object.get(input.spec.template.spec, "securityContext", {}), "runAsNonRoot", false) == true +} +container_run_as_non_root(container) if { + object.get(object.get(container, "securityContext", {}), "runAsNonRoot", null) == true +} +container_run_as_non_root(container) if { + object.get(object.get(container, "securityContext", {}), "runAsNonRoot", null) == null + pod_run_as_non_root_default +} + +deny contains msg if { + input.kind == "Deployment" + container := input.spec.template.spec.containers[_] + not container_run_as_non_root(container) + msg := sprintf("Deployment %v container %v must set securityContext.runAsNonRoot to true or inherit it from the pod securityContext", [input.metadata.name, container.name]) +} + +deny contains msg if { + input.kind == "Deployment" + container := input.spec.template.spec.initContainers[_] + not container_run_as_non_root(container) + msg := sprintf("Deployment %v initContainer %v must set securityContext.runAsNonRoot to true or inherit it from the pod securityContext", [input.metadata.name, container.name]) +} \ No newline at end of file diff --git a/policy/roles.rego b/policy/roles.rego new file mode 100644 index 0000000..00dab38 --- /dev/null +++ b/policy/roles.rego @@ -0,0 +1,58 @@ +package main + +import rego.v1 + +# Roles are namespace-scoped resources. A Role without a namespace will be +# silently defaulted by the API server, which can result in permissions being +# granted in an unintended namespace. Require every Role to declare its +# namespace explicitly so intent is clear. +deny contains msg if { + input.kind == "Role" + not input.metadata.namespace + msg := sprintf("Role %v must have a namespace set", [input.metadata.name]) +} + +# RoleBinding resources, similarly, should be namespace-scoped. +deny contains msg if { + input.kind == "RoleBinding" + not input.metadata.namespace + msg := sprintf("RoleBinding %v must have a namespace set", [input.metadata.name]) +} + +# ClusterRole resources must not have a namespace applied. This is typically ignored, but good hygiene +# to omit to avoid confusion. +deny contains msg if { + input.kind == "ClusterRole" + input.metadata.namespace + msg := sprintf("ClusterRole %v must not have a namespace set", [input.metadata.name]) +} + +# ClusterRoleBinding resources must not have a namespace applied. This is typically ignored, but good hygiene +# to omit to avoid confusion. +deny contains msg if { + input.kind == "ClusterRoleBinding" + input.metadata.namespace + msg := sprintf("ClusterRoleBinding %v must not have a namespace set", [input.metadata.name]) +} + +# A ClusterRoleBinding must not be bound to a Role resource +deny contains msg if { + input.kind == "ClusterRoleBinding" + input.roleRef.kind == "Role" + msg := sprintf("ClusterRoleBinding %v must reference a ClusterRole, not a Role", [input.metadata.name]) +} + +# A RoleBinding must not be bound to a ClusterRole resource +deny contains msg if { + input.kind == "RoleBinding" + input.roleRef.kind == "ClusterRole" + msg := sprintf("RoleBinding %v must reference a Role, not a ClusterRole", [input.metadata.name]) +} + +deny contains msg if { + input.kind in {"RoleBinding", "ClusterRoleBinding"} + subject := input.subjects[_] + subject.kind == "ServiceAccount" + not subject.namespace + msg := sprintf("%v %v has ServiceAccount subject %v without a namespace", [input.kind, input.metadata.name, subject.name]) +} \ No newline at end of file diff --git a/policy/serviceaccounts.rego b/policy/serviceaccounts.rego new file mode 100644 index 0000000..79b7d6a --- /dev/null +++ b/policy/serviceaccounts.rego @@ -0,0 +1,13 @@ +package main + +import rego.v1 + +# ServiceAccounts are namespace-scoped resources. A ServiceAccount without a namespace will be +# silently defaulted by the API server, which can result in permissions being +# granted in an unintended namespace. Require every ServiceAccount to declare its +# namespace explicitly so intent is clear. +deny contains msg if { + input.kind == "ServiceAccount" + not input.metadata.namespace + msg := sprintf("ServiceAccount %v must have a namespace set", [input.metadata.name]) +} \ No newline at end of file