Conversation
2.5.0: CA Bundle with ConfigMap + GKE Ambient Credentials Documentation
* feat: release 2.5.0 (#62) 2.5.0: CA Bundle with ConfigMap + GKE Ambient Credentials Documentation Co-authored-by: Matthew H. Irby <irby@users.noreply.github.com> * feat: add client caching to reduce OAuth token requests Previously, every certificate request reconciliation created a new Command API client, which meant a new OAuth token was fetched for each request. For customers with OAuth provider quotas, this caused rate limiting issues. This change introduces a ClientCache that: - Caches Command API clients by configuration hash - Reuses cached clients across reconciliations for the same issuer - Allows the underlying oauth2 library's token caching to work as intended - Is thread-safe for concurrent reconciliations The cache key is a SHA-256 hash of all configuration fields that affect the client connection (hostname, API path, credentials, scopes, etc.), ensuring different issuers get different clients while the same issuer reuses its client. Fixes: OAuth token re-authentication on every request Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(scripts): update scripting usability * feat: update keyfactor-auth-client-go to v1.3.1 Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore: remove test short circuit Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Revert "Potential fix for pull request finding" This reverts commit 19bc19b. * chore: cleanup Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore: break build & test into its own workflow Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * fix: remove lint from CI Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore(docs): update CHANGELOG Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> --------- Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> Co-authored-by: Morgan Gangwere <470584+indrora@users.noreply.github.com> Co-authored-by: Matthew H. Irby <irby@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Matthew H. Irby <matt.irby@keyfactor.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: log errors from Enrollment API call Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * fix: add missing namespaces, add linting to catch issues Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * feat: add linting Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore: address lint issues Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore: update CHANGELOG Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore: apply copilot feedback Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * feat: fix typo Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * feat: copilot suggestions Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> --------- Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Automated merge of the release-2.5 line into main, bringing in new CA bundle configuration options, client caching for Command API usage, Helm/RBAC/policy enforcement updates, and expanded docs/e2e tooling.
Changes:
- Add CA bundle support via ConfigMap and optional key selection, plus related docs and e2e coverage.
- Introduce a thread-safe Command client cache to reuse OAuth-authenticated clients across reconciliations.
- Add Conftest/Rego policies and CI/Makefile wiring to lint rendered Helm manifests, plus Helm chart namespace/RBAC/template improvements.
Reviewed changes
Copilot reviewed 46 out of 48 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| policy/serviceaccounts.rego | Conftest policy enforcing explicit namespace on ServiceAccounts. |
| policy/roles.rego | Conftest policy enforcing namespace hygiene and roleRef constraints for RBAC objects. |
| policy/deployments.rego | Conftest policy enforcing runAsNonRoot on Deployment containers/initContainers. |
| internal/controller/issuer_controller_test.go | Renames api import alias and adds unit tests for commandConfigFromIssuer (including CA bundle scenarios). |
| internal/controller/issuer_controller.go | Adds CA bundle ConfigMap/key support and improves error wrapping in reconcile/config-building. |
| internal/controller/certificaterequest_controller_test.go | Removes unused test certificate helper and related imports. |
| internal/controller/certificaterequest_controller.go | Improves error wrapping and adds a success log on signing. |
| internal/command/command_test.go | Updates fakes and tests for new client API behavior; adds response-body-in-error coverage. |
| internal/command/command.go | Updates metadata-field fetch API usage; improves error detail gathering and closes HTTP bodies in enrollment-pattern lookup. |
| internal/command/client_test.go | Updates tests to reflect printClaims no longer returning an error. |
| internal/command/client_cache_test.go | Adds tests for config hashing and basic cache operations. |
| internal/command/client_cache.go | Adds cached signer/health-checker creation keyed by a config hash. |
| internal/command/client.go | Updates Client interface for metadata field calls, adds Azure token fetch timeout, and changes printClaims to log-only. |
| go.sum | Bumps keyfactor-auth-client-go checksums to v1.3.1. |
| go.mod | Bumps keyfactor-auth-client-go to v1.3.1 and adjusts Go version directive. |
| e2e/run_tests.sh | Makes image/chart configuration env-overridable; adds CA secret/configmap generation and CA bundle tests; supports non-minikube context. |
| e2e/certs/.gitkeep | Adds placeholder for CA cert material used by e2e. |
| e2e/README.md | Expands e2e setup docs, variables, and cluster/minikube usage guidance. |
| e2e/.gitignore | Ignores .env and cert material while keeping .gitkeep. |
| e2e/.env.example | Adds DISABLE_CA_CHECK and keeps oauth audience example. |
| docsource/content.md | Updates install guidance, adds GKE ambient creds link, and moves CA bundle section to new docs. |
| docs/ca-bundle/README.md | Adds dedicated CA bundle documentation incl. trust-manager guidance. |
| docs/ambient-providers/google.md | Adds extensive documentation for GKE workload identity ambient auth. |
| deploy/charts/command-cert-manager-issuer/values.yaml | Adds configmap-access toggle and env injection support. |
| deploy/charts/command-cert-manager-issuer/templates/serviceaccount.yaml | Sets ServiceAccount namespace explicitly. |
| deploy/charts/command-cert-manager-issuer/templates/rolebinding.yaml | Sets namespaces for RoleBindings and adds configmap-reader binding. |
| deploy/charts/command-cert-manager-issuer/templates/role.yaml | Sets namespaces for Roles and adds configmap-reader role. |
| deploy/charts/command-cert-manager-issuer/templates/deployment.yaml | Adds configmap access flag wiring and supports user-defined env vars. |
| deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml | Adds CRD fields for caBundleConfigMapName and caBundleKey. |
| deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml | Adds CRD fields for caBundleConfigMapName and caBundleKey. |
| deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml | Adds clusterissuers/finalizers update permission. |
| deploy/charts/command-cert-manager-issuer/ci/default-values.yaml | Adds CI values set to exercise default RBAC paths. |
| deploy/charts/command-cert-manager-issuer/ci/cluster-access-values.yaml | Adds CI values set to exercise cluster-wide RBAC paths. |
| deploy/charts/command-cert-manager-issuer/README.md | Documents new env chart value. |
| config/crd/bases/command-issuer.keyfactor.com_issuers.yaml | Generated CRD updated with CA bundle configmap/key fields. |
| config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml | Generated CRD updated with CA bundle configmap/key fields. |
| cmd/main.go | Adds configmap access flag, scoped cache setup, and shared Command client cache usage. |
| api/v1alpha1/issuer_types_test.go | Fixes time comparison usage for transition time. |
| api/v1alpha1/issuer_types.go | Adds IssuerSpec CA bundle configmap/key fields and simplifies condition unsetting logic. |
| README.md | Updates install guidance, adds GKE ambient creds link, and moves CA bundle section to new docs. |
| Makefile | Updates e2e target to shell script, adds helm/conftest manifest linting targets, and tool installer updates. |
| Dockerfile | Minor whitespace-only change near module copy layer. |
| CONTRIBUTING.md | Adds contribution guidance incl. manifest linting and e2e pointers. |
| CHANGELOG.md | Adds entries for v2.5.0–v2.5.2. |
| .kube-linter.yaml | Adds kube-linter config (all built-ins enabled). |
| .golangci.yml | Migrates config to v2 format, updates enabled linters/formatters and exclusions. |
| .github/workflows/test.yml | Adds new Build/Test/Lint workflow with CRD drift + Helm manifest linting. |
| .github/workflows/keyfactor-bootstrap-workflow.yml | Removes legacy build/test jobs in favor of calling starter workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+299
to
+303
| } else { | ||
| // If no caBundleKey is specified, take the last entry in the caData map | ||
| for _, bytes := range caData { | ||
| caCertBytes = bytes | ||
| } |
Comment on lines
+87
to
+122
| // GetOrCreateSigner returns a cached signer for the given config, or creates | ||
| // a new one if none exists. This ensures OAuth tokens are reused across | ||
| // requests to the same Command instance. | ||
| func (c *ClientCache) GetOrCreateSigner(ctx context.Context, config *Config) (Signer, error) { | ||
| key := configHash(config) | ||
| logger := log.FromContext(ctx) | ||
|
|
||
| // Fast path: check if we have a cached client | ||
| c.mu.RLock() | ||
| if cached, ok := c.clients[key]; ok { | ||
| c.mu.RUnlock() | ||
| logger.V(1).Info("Reusing cached Command client", "cacheKey", key[:12]) | ||
| return cached.signer, nil | ||
| } | ||
| c.mu.RUnlock() | ||
|
|
||
| // Slow path: create a new client | ||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
|
|
||
| // Double-check after acquiring write lock | ||
| if cached, ok := c.clients[key]; ok { | ||
| logger.V(1).Info("Reusing cached Command client (after lock)", "cacheKey", key[:12]) | ||
| return cached.signer, nil | ||
| } | ||
|
|
||
| logger.Info("Creating new Command client (will be cached for future requests)", "cacheKey", key[:12]) | ||
|
|
||
| s, err := newInternalSigner(ctx, config, commandsdk.NewAPIClient) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create signer: %w", err) | ||
| } | ||
|
|
||
| c.clients[key] = &cachedClient{signer: s} | ||
| return s, nil | ||
| } |
Comment on lines
+963
to
+965
| echo """🔐 Creating CA secret used for testing..." | ||
| regenerate_ca_secret | ||
| regenerate_ca_config_map |
|
|
||
| ## Using Self-Signed Certificates | ||
|
|
||
| If the targeted Keyfactor Command API is configured to use a self-signed certificate or with a certificate whose issuer isn't widely trusted, the CA certificate **must be provided** via a Kubernetes Secret of ConfigMap. The secret must belong to the same namespace that command-cert-manager-issuer is deployed to (i.e. `command-issuer-system`). |
Comment on lines
62
to
+69
| 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"}) |
Comment on lines
+63
to
+71
| | Variable | Description | Default | | ||
| |----------|-------------|---------| | ||
| | `IMAGE_TAG` | Docker image version to test | `2.5.0` | | ||
| | `HELM_CHART_VERSION` | Helm chart version | `2.5.0` | | ||
| | `E2E_ENROLLMENT_PATTERN_NAME` | Enrollment pattern name | `Default Pattern` | | ||
| | `E2E_OWNER_ROLE_NAME` | Owner role name | `InstanceOwner` | | ||
| | `DISABLE_CA_CHECK` | Skip TLS CA verification | `false` | | ||
| | `USE_MINIKUBE` | Use minikube instead of current kubeconfig | `false` | | ||
| | `IMAGE_REGISTRY` | Registry to push local builds (when `IMAGE_TAG=local`) | - | |
Comment on lines
+261
to
+266
| if issuer.GetSpec().CaBundleConfigMapName != "" { | ||
| var configMap corev1.ConfigMap | ||
| err := c.Get(ctx, types.NamespacedName{ | ||
| Name: issuer.GetSpec().CaBundleConfigMapName, | ||
| Namespace: secretNamespace, | ||
| }, &configMap) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge release-2.5 to main - Automated PR