Merge 2.5.3 to main#69
Open
indrora wants to merge 4 commits into
Open
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>
* 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> * Merge 2.5.1 to main (#65) * feat: release 2.5.0 2.5.0: CA Bundle with ConfigMap + GKE Ambient Credentials Documentation * release: 2.5.1 * 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> --------- Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> Co-authored-by: Matthew H. Irby <irby@users.noreply.github.com> Co-authored-by: spb <1661003+spbsoluble@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> * Merge 2.5.2 to main (#67) * feat: release 2.5.0 2.5.0: CA Bundle with ConfigMap + GKE Ambient Credentials Documentation * release: 2.5.1 * 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> * Add missing namespace specification + address linting issues (#66) * 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> --------- Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> Co-authored-by: Matthew H. Irby <irby@users.noreply.github.com> Co-authored-by: spb <1661003+spbsoluble@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> * chore(deps): patch vulnerable dependencies Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * fix(tests): fix test failures caused by merge conflict resolution issue Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore(docs): update CHANGELOG Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore(ci): update trigger for dependency review Signed-off-by: Matthew H. Irby <matt.irby@keyfactor.com> * chore(ci): address copilot feedback 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: spb <1661003+spbsoluble@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This automated merge brings the release-2.5 (v2.5.3) changes into main, including new CA bundle configuration options, client/token reuse improvements, policy-based Helm manifest linting, documentation updates, and dependency/security upgrades.
Changes:
- Add CA bundle support via ConfigMap and an optional key selector, updating controller logic, CRDs, Helm chart, docs, and e2e tests accordingly.
- Introduce a reusable Command client cache for OAuth/token reuse and improve error wrapping/logging.
- Add CI workflow + Conftest Rego policies for Helm-rendered manifest linting; update dependencies for security fixes.
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 |
|---|---|
| README.md | Documents Helm version installs, CA bundle options, and GKE ambient auth. |
| policy/serviceaccounts.rego | Conftest policy requiring explicit namespaces on ServiceAccounts. |
| policy/roles.rego | Conftest policies for Role/Binding namespace hygiene and roleRef/subjects validation. |
| policy/deployments.rego | Conftest policy enforcing runAsNonRoot for Deployment containers/initContainers. |
| Makefile | Adds manifest linting targets, tool installers (conftest/kube-linter), updates golangci-lint install/version, and updates e2e runner target. |
| internal/controller/issuer_controller.go | Adds CA bundle loading from ConfigMap, optional bundle key selection, and improves error wrapping. |
| internal/controller/issuer_controller_test.go | Updates API import alias and adds unit tests for commandConfigFromIssuer (including CA bundle cases). |
| internal/controller/certificaterequest_controller.go | Improves error wrapping and adds success log on signing completion. |
| internal/controller/certificaterequest_controller_test.go | Removes unused certificate helper/imports. |
| internal/command/command.go | Adjusts client API usage, improves enrollment failure details, and refactors enrollment pattern lookup. |
| internal/command/command_test.go | Updates fake client interface, adds coverage for error response-body inclusion, and adjusts CSR generation invocation. |
| internal/command/client.go | Changes GetAllMetadataFields interface to close response internally; adds Azure token fetch timeout; adjusts JWT claim logging helper. |
| internal/command/client_test.go | Updates tests to match printClaims signature change (no longer returns error). |
| internal/command/client_cache.go | Adds a thread-safe signer/healthchecker cache keyed by a config hash. |
| internal/command/client_cache_test.go | Adds unit tests for hash determinism and basic cache operations. |
| go.mod | Updates Go/module versions (notably security-related dependency bumps). |
| go.sum | Updates dependency checksums corresponding to go.mod changes. |
| e2e/run_tests.sh | Makes e2e configurable via env vars, supports non-minikube clusters, and adds CA Secret/ConfigMap test coverage. |
| e2e/README.md | Expands e2e documentation, requirements, env var reference, and execution modes. |
| e2e/certs/.gitkeep | Keeps certs directory tracked for CA bundle inputs. |
| e2e/.gitignore | Ignores .env and cert contents while preserving .gitkeep. |
| e2e/.env.example | Adds DISABLE_CA_CHECK example variable. |
| docsource/content.md | Mirrors README install/CA bundle documentation updates for doc generation. |
| docs/ca-bundle/README.md | New CA bundle documentation, including trust-manager guidance and security considerations. |
| docs/ambient-providers/google.md | New GKE ambient credentials/workload identity documentation. |
| Dockerfile | Minor formatting change (extra blank lines). |
| deploy/charts/command-cert-manager-issuer/values.yaml | Adds configmap access option and pod env injection values. |
| deploy/charts/command-cert-manager-issuer/templates/serviceaccount.yaml | Ensures ServiceAccount has explicit namespace. |
| deploy/charts/command-cert-manager-issuer/templates/rolebinding.yaml | Adds explicit namespace handling and adds configmap reader binding (Role/ClusterRole binding). |
| deploy/charts/command-cert-manager-issuer/templates/role.yaml | Adds explicit namespace handling and adds configmap reader role (Role/ClusterRole). |
| deploy/charts/command-cert-manager-issuer/templates/deployment.yaml | Adds flag for configmap access and supports injecting environment variables. |
| deploy/charts/command-cert-manager-issuer/templates/crds/issuers.yaml | Adds caBundleConfigMapName and caBundleKey to Issuer CRD schema/docs. |
| deploy/charts/command-cert-manager-issuer/templates/crds/clusterissuers.yaml | Adds caBundleConfigMapName and caBundleKey to ClusterIssuer CRD schema/docs. |
| deploy/charts/command-cert-manager-issuer/templates/clusterrole.yaml | Adds finalizers permission for clusterissuers. |
| deploy/charts/command-cert-manager-issuer/README.md | Documents new env Helm value. |
| deploy/charts/command-cert-manager-issuer/ci/default-values.yaml | Adds CI values file for namespace-scoped RBAC mode. |
| deploy/charts/command-cert-manager-issuer/ci/cluster-access-values.yaml | Adds CI values file for cluster-scoped RBAC mode. |
| CONTRIBUTING.md | Adds contributor guidance including manifest linting and e2e instructions. |
| config/crd/bases/command-issuer.keyfactor.com_issuers.yaml | Updates generated CRD to include CA bundle ConfigMap/key fields. |
| config/crd/bases/command-issuer.keyfactor.com_clusterissuers.yaml | Updates generated CRD to include CA bundle ConfigMap/key fields. |
| cmd/main.go | Adds configmap access scoping, initializes shared client cache, and wires cache into controllers/builders. |
| CHANGELOG.md | Adds v2.5.0–v2.5.3 entries including security bumps and new features. |
| api/v1alpha1/issuer_types.go | Adds new IssuerSpec fields for CA bundle ConfigMap/key and tweaks condition removal logic. |
| api/v1alpha1/issuer_types_test.go | Fixes time assertion to use the correct metav1.Time accessor. |
| .kube-linter.yaml | Adds kube-linter configuration scaffold. |
| .golangci.yml | Migrates/updates golangci-lint configuration for v2 and enables goimports as a formatter. |
| .github/workflows/test.yml | Adds build/test/lint workflow including CRD drift check and manifest linting. |
| .github/workflows/keyfactor-bootstrap-workflow.yml | Removes prior build/test steps and relies on the starter workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```shell | ||
| helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ | ||
| --namespace command-issuer-system \ | ||
| --version 2.4.0 |
Comment on lines
+164
to
+165
| > A list of configurable Helm chart parameters can be found [in the Helm chart docs](./deploy/charts/command-cert-manager-issuer/README.md#configuration) | ||
|
|
| ```shell | ||
| helm install command-cert-manager-issuer command-issuer/command-cert-manager-issuer \ | ||
| --namespace command-issuer-system \ | ||
| --version 2.4.0 |
Comment on lines
+141
to
+155
| if !secretAccessGrantedAtClusterLevel { | ||
| setupLog.Info(fmt.Sprintf("expecting SA to have Get+List+Watch permissions for corev1 Secret resources in the %q namespace", clusterResourceNamespace)) | ||
| byObject[&corev1.Secret{}] = cache.ByObject{ | ||
| Namespaces: map[string]cache.Config{clusterResourceNamespace: {}}, | ||
| } | ||
| } else { | ||
| setupLog.Info("expecting SA to have Get+List+Watch permissions for corev1 Secret resources at cluster level") | ||
| } | ||
|
|
||
| if !configMapAccessGrantedAtClusterLevel { | ||
| setupLog.Info(fmt.Sprintf("expecting SA to have Get+List+Watch permissions for corev1 ConfigMap resources in the %q namespace", clusterResourceNamespace)) | ||
| byObject[&corev1.ConfigMap{}] = cache.ByObject{ | ||
| Namespaces: map[string]cache.Config{clusterResourceNamespace: {}}, | ||
| } | ||
| } else { |
Comment on lines
+293
to
304
| if issuer.GetSpec().CaBundleKey != "" { | ||
| caCert, ok := caData[issuer.GetSpec().CaBundleKey] | ||
| if !ok { | ||
| return nil, fmt.Errorf("%w: caBundleKey '%s' not found in CA bundle data", errGetCaBundleKey, issuer.GetSpec().CaBundleKey) | ||
| } | ||
| caCertBytes = caCert | ||
| } else { | ||
| // If no caBundleKey is specified, take the last entry in the caData map | ||
| for _, bytes := range caData { | ||
| caCertBytes = bytes | ||
| } | ||
| } |
Comment on lines
+63
to
+66
| | Variable | Description | Default | | ||
| |----------|-------------|---------| | ||
| | `IMAGE_TAG` | Docker image version to test | `2.5.0` | | ||
| | `HELM_CHART_VERSION` | Helm chart version | `2.5.0` | |
|
|
||
| ## 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`). |
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