using credentials for all gitops config options - #561
Closed
ThomasMichael1811 wants to merge 1 commit into
Closed
ThomasMichael1811 wants to merge 1 commit into
ThomasMichael1811 wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Central SCM configs can now expose null credentials to providers (breaking auth), and the repo adds plaintext secret values that should be replaced with placeholders before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR expands GitOps Playground configuration to support referencing credentials via Kubernetes Secrets across multiple config sections, and wires a pre-config lifecycle hook to resolve those references before tools run.
Changes:
- Add
Credentialsreference fields to config schemas (application/jenkins/registry/scm) and generate updated JSON schema/docs. - Introduce
CommonToolConfigsecret-resolution inpreConfigInitusingK8sClient.getCredentialsFromSecret(Credentials). - Update/extend test coverage and add dev helper manifests/Makefile target for local secret-based testing.
File summaries
| File | Description |
|---|---|
| src/test/groovy/com/cloudogu/gitops/tools/core/scmmanager/ScmManagerSetupTest.groovy | Adjusts SCM Manager setup test config to reflect new credentials handling. |
| src/test/groovy/com/cloudogu/gitops/infrastructure/kubernetes/api/K8sClientTest.groovy | Updates tests to use Credentials object-based secret resolution API. |
| src/test/groovy/com/cloudogu/gitops/config/schema/CredentialsDelegationTest.groovy | New tests validating Credentials constructors/copy behavior and schema delegation. |
| src/test/groovy/com/cloudogu/gitops/config/schema/ConfigTest.groovy | Reformatting/cleanup in config tests. |
| src/test/groovy/com/cloudogu/gitops/cli/ApplicationConfiguratorTest.groovy | Updates hook/config wiring to new CommonToolConfig(K8sClient) constructor. |
| src/main/java/com/cloudogu/gitops/tools/common/CommonToolConfig.java | Adds secret-based credentials extraction during preConfigInit. |
| src/main/java/com/cloudogu/gitops/infrastructure/kubernetes/api/K8sClient.java | Consolidates secret credential resolution around Credentials input (with namespace defaulting). |
| src/main/java/com/cloudogu/gitops/config/scm/ScmTenantSchema.java | Adds optional credentials field and ensures tenant SCM configs can provide credentials via reference or plain values. |
| src/main/java/com/cloudogu/gitops/config/scm/ScmCentralSchema.java | Adds optional credentials field to central SCM configs (GitLab/SCM-Manager). |
| src/main/java/com/cloudogu/gitops/config/Credentials.java | Extends copy constructor; introduces isUsed() helper for secret-reference detection. |
| src/main/java/com/cloudogu/gitops/config/Config.java | Adds credentials fields to application/jenkins/registry schema sections. |
| src/main/java/com/cloudogu/gitops/cli/GitopsPlaygroundCli.java | Passes K8sClient into lifecycle hooks and instantiates CommonToolConfig with it. |
| src/main/java/com/cloudogu/gitops/application/Application.java | Minor formatting-only change. |
| scripts/dev/gop-secrets.yaml | Adds example Kubernetes Secrets for local/dev testing. |
| scripts/dev/gop-secrets-values.yaml | Adds example config values demonstrating secret references. |
| Makefile | Adds a helper target to init cluster and apply dev secrets. |
| docs/Developers.md | Updates developer docs for local image usage in example command. |
| docs/configuration.schema.json | Updates generated schema to include new credentials objects. |
| docs/Configuration.md | Updates configuration reference docs to include new credentials paths. |
Review details
Suppressed comments (2)
scripts/dev/gop-secrets.yaml:19
- This repository file contains literal secret values (stringData.password). Even for dev tooling, committing non-placeholder passwords increases the risk of accidental reuse/leakage. Replace with obvious placeholders.
stringData:
username: admin
password: who_can_read_this
scripts/dev/gop-secrets.yaml:29
- This repository file contains literal secret values (stringData.password). Even for dev tooling, committing non-placeholder passwords increases the risk of accidental reuse/leakage. Replace with obvious placeholders.
stringData:
username: myregistry
password: mypassword
- Files reviewed: 19/19 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
49
to
52
|
|
||
| public void setCredentials(Credentials credentials) { | ||
| this.credentials = credentials; | ||
| } |
Comment on lines
+82
to
+84
| @JsonPropertyDescription(CENTRAL_SCMM_USERNAME_DESCRIPTION) | ||
| private Credentials credentials; | ||
|
|
ThomasMichael1811
force-pushed
the
feature/gop-secrets
branch
4 times, most recently
from
September 7, 2026 11:32
e7d66a1 to
0228a79
Compare
ThomasMichael1811
force-pushed
the
feature/gop-secrets
branch
from
September 7, 2026 12:13
0228a79 to
0c2c1a8
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed correctness and robustness issues in secret-credential resolution (including misleading schema/docs annotations and error-handling gaps) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (5)
src/main/java/com/cloudogu/gitops/config/Credentials.java:95
isUsed()currently requiressecretNamespace != null, which prevents secret-based credentials resolution when users specify onlysecretName(YAML omission -> null). This also contradictsK8sClient#getCredentialsFromSecret, which defaults the namespace todefaultwhen missing/blank. Consider treatingsecretNamespaceas optional and basing "used" on a non-blanksecretName.
/**
* ensures that secretName and secretNamespace are not null
*
* @return true, if User sets credentials with secretName and secretNamespace otherwise false.
*/
@JsonIgnore
public boolean isUsed() {
return secretName != null && secretNamespace != null;
}
src/main/java/com/cloudogu/gitops/config/scm/ScmTenantSchema.java:125
- The
credentialsfield is annotated withSCMM_USERNAME_DESCRIPTION("Mandatory when scmm-url is set"), which is unrelated to secret-based credentials and will generate misleading schema/docs.
@JsonPropertyDescription(SCMM_USERNAME_DESCRIPTION)
private Credentials credentials;
src/main/java/com/cloudogu/gitops/config/scm/ScmCentralSchema.java:83
- The
credentialsfield is annotated withCENTRAL_SCMM_USERNAME_DESCRIPTION, which describes a username string rather than secret-based credentials; this will produce misleading schema/docs.
@JsonPropertyDescription(CENTRAL_SCMM_USERNAME_DESCRIPTION)
private Credentials credentials;
scripts/dev/gop-secrets.yaml:9
- This file hardcodes example usernames/passwords directly in a committed Secret manifest. Even for dev helpers, it's safer to use clearly non-sensitive placeholders (e.g.,
CHANGEME) to reduce the risk of accidental reuse/copy-paste into real environments.
stringData:
username: admin
password: this_is_for_your_ads
scripts/dev/gop-secrets.yaml:29
- Same issue as above: avoid committing non-placeholder password values in example Secret manifests to reduce the chance of accidental reuse.
stringData:
username: myregistry
password: mypassword
- Files reviewed: 19/19 changed files
- Comments generated: 6
- Review effort level: Lite
Comment on lines
+39
to
+41
| ./scripts/init-cluster.sh | ||
| kubectl create namespace gop-job | ||
| kubectl apply -f ./scripts/dev/gop-secrets.yaml |
Comment on lines
+479
to
+480
| @JsonPropertyDescription(JENKINS_METRICS_PASSWORD_DESCRIPTION) | ||
| private Credentials metricsCredentials; |
Comment on lines
+38
to
+40
| @JsonPropertyDescription(CENTRAL_GITLAB_URL_DESCRIPTION) | ||
| private Credentials credentials; | ||
|
|
Comment on lines
+74
to
+75
| @JsonPropertyDescription(GITLAB_URL_DESCRIPTION) | ||
| private Credentials credentials; |
Comment on lines
+552
to
+572
| String namespace = (credentials.getSecretNamespace() != null && !credentials.getSecretNamespace().trim().isEmpty()) | ||
| ? credentials.getSecretNamespace() | ||
| : "default"; | ||
| Secret secret = client.secrets() | ||
| .inNamespace(namespace) | ||
| .withName(credentials.getSecretName()) | ||
| .get(); | ||
| if (secret == null || secret.getData() == null) { | ||
| throw new IllegalStateException("Secret " + credentials.getSecretName() + NOT_FOUND_IN_NAMESPACE + namespace); | ||
| } | ||
|
|
||
| Map<String, String> secretData = secret.getData(); | ||
| String usernameEncoded = secretData.get(credentials.getUsernameKey()); | ||
| String username = usernameEncoded != null ? new String( | ||
| Base64.getDecoder() | ||
| .decode(usernameEncoded), StandardCharsets.UTF_8 | ||
| ) : credentials.getUsername(); | ||
| String password = new String( | ||
| Base64.getDecoder() | ||
| .decode(secretData.get(credentials.getPasswordKey())), StandardCharsets.UTF_8 | ||
| ); |
Comment on lines
+32
to
+36
| | - | `registry.credentials.username` | String | `-` | Credentials Object to authenticate against content repo. Allows using a K8s Secret | | ||
| | - | `registry.credentials.secretNamespace` | String | `-` | Credentials Object to authenticate against content repo. Allows using a K8s Secret | | ||
| | - | `registry.credentials.secretName` | String | `-` | Credentials Object to authenticate against content repo. Allows using a K8s Secret | | ||
| | - | `registry.credentials.usernameKey` | String | `-` | Credentials Object to authenticate against content repo. Allows using a K8s Secret | | ||
| | - | `registry.credentials.passwordKey` | String | `-` | Credentials Object to authenticate against content repo. Allows using a K8s Secret | |
Contributor
Author
|
nor more neccessary |
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.
Pull request overview
This PR expands GitOps Playground configuration to support referencing credentials via Kubernetes Secrets across multiple config sections, and wires a pre-config lifecycle hook to resolve those references before tools run.
Changes:
Credentialsreference fields to config schemas (application/jenkins/registry/scm) and generate updated JSON schema/docs.CommonToolConfigsecret-resolution inpreConfigInitusingK8sClient.getCredentialsFromSecret(Credentials).