feat: support extra static fields in managed Secrets#120
Open
isometry wants to merge 2 commits into
Open
Conversation
isometry
force-pushed
the
feat/secret-extra-data
branch
2 times, most recently
from
0037064 to
7674877
Compare
Add spec.secret.extraData to Token and ClusterToken as an ordered list of inline, configMap, or secret projection sources merged into the generated Secret. Refs support an optional key allowlist and an optional flag controlling whether a missing/unreadable source fails the reconcile (deleting the managed Secret) or is skipped. Operator-managed keys (token, or username/password under basicAuth) always win; collisions and dropped reserved keys are surfaced as Warning events. Inline reserved-key and key-name violations are rejected at admission via CEL. Adds e2e coverage for the merge, optional-skip, and fail-closed paths. Also bumps golangci-lint v2.1.0 -> v2.12.2 (the old binary is built with Go 1.25 and refuses go 1.26.4) and resolves the findings its newer linters surfaced.
isometry
force-pushed
the
feat/secret-extra-data
branch
from
7674877 to
bc8f427
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for projecting additional static key/value pairs into operator-managed Secrets via spec.secret.extraData on Token and ClusterToken, including admission-time validation of reserved keys and reconcile-time handling of referenced sources.
Changes:
- Implement
extraDataresolution/merge logic (inline + referenced ConfigMaps/Secrets), including reserved-key handling, warning events, and fail-closed behavior for required sources. - Extend CRD schemas + Helm chart CRD templates/README and update RBAC to allow reading ConfigMaps.
- Add unit + e2e coverage for
extraData, and bump GitHub client libs/tooling versions.
Reviewed changes
Copilot reviewed 36 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/e2e_test.go | Adds e2e scenarios for extraData merge/optional/required behavior for Token/ClusterToken. |
| test/e2e/e2e_helpers_test.go | Adds helpers for Secret/ConfigMap fixtures + warning-event waits; updates GitHub client usage. |
| README.md | Documents spec.secret.extraData behavior and semantics. |
| Makefile | Updates golangci-lint version used by the build tooling. |
| internal/tokenmanager/token_secret.go | Resolves extraData early in reconcile and merges it into managed Secret data. |
| internal/tokenmanager/token_secret_test.go | Unit-tests that managed credential keys override resolved extraData. |
| internal/tokenmanager/token_manager.go | Extends TokenManager interface for extraData sources. |
| internal/tokenmanager/extradata.go | Implements projection/allowlist/optional vs required behavior and warning events. |
| internal/tokenmanager/extradata_test.go | Unit-tests extraData projection behavior and warning-event emission. |
| internal/metrics/recorder.go | Adds a reconcile error reason for extra_data. |
| internal/ghapp/registry.go | Updates ghait import and refines registry error message. |
| internal/ghapp/registry_test.go | Updates go-github/ghait imports for tests. |
| internal/ghapp/providers.go | Updates provider imports to ghait v88. |
| internal/ghapp/providers_test.go | Updates ghait provider imports for tests. |
| internal/controller/token_controller.go | Adds kubebuilder RBAC for ConfigMap get. |
| internal/controller/reconcile_token.go | Wires APIReader + EventRecorder into tokenmanager options. |
| internal/controller/clustertoken_controller.go | Adds kubebuilder RBAC for ConfigMap get. |
| internal/controller/appresolver.go | Updates ghait import version. |
| internal/controller/appconfig.go | Improves referenced-Secret error messages. |
| go.sum | Updates dependency checksums for bumped modules. |
| go.mod | Updates dependency versions and Go version directive. |
| deploy/charts/github-token-manager/templates/rbac.yaml | Grants ConfigMap get in Helm chart RBAC. |
| deploy/charts/github-token-manager/templates/crds.yaml | Adds extraData schema + CEL validations to rendered CRDs. |
| config/rbac/role.yaml | Grants ConfigMap get in controller-manager ClusterRole. |
| config/crd/bases/github.as-code.io_tokens.yaml | Adds extraData schema + validations to Token CRD base. |
| config/crd/bases/github.as-code.io_clustertokens.yaml | Adds extraData schema + validations to ClusterToken CRD base. |
| cmd/manager/main.go | Refactors field index setup; wires APIReader + EventRecorder into reconcilers. |
| api/v1/zz_generated.deepcopy.go | Adds deepcopy support for new API types/fields (ExtraData, SecretDataSource*). |
| api/v1/token_types.go | Adds ExtraData field + CEL validations; implements GetSecretDataSources(). |
| api/v1/token_types_test.go | Tests Token GetSecretDataSources() normalization behavior. |
| api/v1/secretdatasource.go | Introduces SecretDataSource / SecretDataSourceRef and normalization helper. |
| api/v1/permissions.go | Updates go-github import version. |
| api/v1/permissions_test.go | Removes stray helper; keeps tests aligned with updated deps. |
| api/v1/helpers_test.go | Updates go-github import version. |
| api/v1/groupversion_info.go | Adds nolint for deprecated kubebuilder scaffolding type. |
| api/v1/clustertoken_types.go | Adds ExtraData field + CEL validations; implements GetSecretDataSources(). |
| api/v1/clustertoken_types_test.go | Tests ClusterToken GetSecretDataSources() defaulting behavior. |
| .golangci.yml | Adjusts linter exclusions for test files. |
Files not reviewed (1)
- api/v1/zz_generated.deepcopy.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rewrite import paths from v84 to v88 and adapt to the new github.NewClient options-pattern constructor, replacing the oauth2 static-token client in the e2e helpers with WithAuthToken.
isometry
force-pushed
the
feat/secret-extra-data
branch
from
37661f4 to
6afb4eb
Compare
pcanilho
reviewed
Jul 3, 2026
| extraData, err := s.resolveExtraData(ctx) | ||
| if err != nil { | ||
| log.Info("required extraData source unavailable, deleting managed secret", "reason", err.Error()) | ||
| if delErr := s.DeleteSecret(ctx, secretKey); delErr != nil { |
Collaborator
There was a problem hiding this comment.
If a source is (temporarily?) unavailable, the whole secret is destroyed. How about taking this chance to report degraded?
P.S.: Unsure whether the last "good" secret should be left in-place in this case as well...?
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.
Add spec.secret.extraData to Token and ClusterToken to merge additional static key/value pairs into the generated Secret. Operator-managed keys (token, or username/password under basicAuth) are reserved and rejected at admission via CEL validation.