Skip to content

fix(controllers): Usage of k8s secret dockerconfigjson#1353

Merged
Skarlso merged 15 commits into
open-component-model:mainfrom
frewilhelm:fix-controller-secret-dockerconfigjson
Dec 10, 2025
Merged

fix(controllers): Usage of k8s secret dockerconfigjson#1353
Skarlso merged 15 commits into
open-component-model:mainfrom
frewilhelm:fix-controller-secret-dockerconfigjson

Conversation

@frewilhelm

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Instead of adding e2e examples with credentials, I added them as a separate tests because the file-parsing and deciding when which action of the ocmcli should be run is hell.
(For example we need to include an .ocmconfig to push to the private registry, but must not copy the resource because then the oci-artifact inside the CV would have the "external" URL which flux does not like. IMO this was not maintainable which is why I added them as e2e tests in testdata. It is more redundant code and configs but its easier maintainable)

Which issue(s) this PR fixes

Fixes open-component-model/ocm-project#788

@frewilhelm frewilhelm force-pushed the fix-controller-secret-dockerconfigjson branch 2 times, most recently from 348cfaf to b523e95 Compare December 5, 2025 19:42
@gitguardian

gitguardian Bot commented Dec 8, 2025

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
- - Generic High Entropy Secret 3bbe3c9 kubernetes/controller/test/e2e/testdata/docker-config-json/.docker-config-json View secret
- - Base64 Generic High Entropy Secret 3bbe3c9 kubernetes/controller/test/e2e/testdata/docker-config-json/bootstrap.yaml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@frewilhelm frewilhelm force-pushed the fix-controller-secret-dockerconfigjson branch 2 times, most recently from b9f376c to a8b9f9e Compare December 8, 2025 14:16
@frewilhelm frewilhelm marked this pull request as ready for review December 8, 2025 14:32
@frewilhelm frewilhelm requested a review from a team as a code owner December 8, 2025 14:32
@frewilhelm frewilhelm force-pushed the fix-controller-secret-dockerconfigjson branch from a6adaa3 to 4ae6880 Compare December 8, 2025 14:37
Skarlso
Skarlso previously approved these changes Dec 8, 2025

@Skarlso Skarlso left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@frewilhelm frewilhelm force-pushed the fix-controller-secret-dockerconfigjson branch from 4ae6880 to 177d912 Compare December 8, 2025 16:13

@jakobmoellerdev jakobmoellerdev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need an e2e and integration test? i feel like one of them should be sufficient.

Comment thread kubernetes/controller/internal/configuration/config.go
Comment thread kubernetes/controller/test/e2e/e2e_credentials_test.go Outdated
@frewilhelm frewilhelm force-pushed the fix-controller-secret-dockerconfigjson branch from c5ae00e to 4887f3d Compare December 9, 2025 07:06
@frewilhelm

Copy link
Copy Markdown
Contributor Author

do we really need an e2e and integration test? i feel like one of them should be sufficient.

I believe the TestGetConfigFromSecret tests are valuable because they help us quickly identify issues with the conversion from an OCM config or docker-config-json to genericv1.Config. For this reason, I’d prefer to keep these tests.

Skarlso
Skarlso previously approved these changes Dec 9, 2025
Comment thread bindings/go/oci/credentials/docker_config.go Outdated
Comment thread kubernetes/controller/internal/configuration/config.go Outdated
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm frewilhelm force-pushed the fix-controller-secret-dockerconfigjson branch from b43bbc6 to 761ae02 Compare December 9, 2025 10:39
frewilhelm and others added 2 commits December 9, 2025 11:50
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Comment thread bindings/go/oci/credentials/docker_config.go Outdated
Skarlso and others added 2 commits December 9, 2025 15:04
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Co-authored-by: Frederic Wilhelm <fre.wilhelm@gmail.com>
Signed-off-by: Gergely Brautigam <skarlso777@gmail.com>
Comment thread bindings/go/oci/credentials/docker_config.go
Comment thread bindings/go/oci/credentials/docker_config.go
@Skarlso

Skarlso commented Dec 10, 2025

Copy link
Copy Markdown
Contributor

How did this suddenly start to fail? :D

@frewilhelm frewilhelm changed the title fix: Fix controller secret dockerconfigjson fix(controllers): Usage of k8s secret dockerconfigjson Dec 10, 2025
@frewilhelm

frewilhelm commented Dec 10, 2025

Copy link
Copy Markdown
Contributor Author

How did this suddenly start to fail? :D

I was able to reproduce it locally ONCE. I ran >20 test-runs afterwards and they all passed. Another run on the GHA also passes.

I suspect that the test fails, if the credential type registration registers the versioned type first and it becomes the default. Then, the expected configuration (unversioned) does not match the returned configuration (versioned in such cases).

I have no idea how this could even happen

@Skarlso Skarlso merged commit 1338087 into open-component-model:main Dec 10, 2025
23 of 24 checks passed
@frewilhelm frewilhelm deleted the fix-controller-secret-dockerconfigjson branch March 30, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ocm-k8s-toolkit - component controller missing implementation handler for .ocmconfigs. when using secrets of type .dockerconfigjson

4 participants