Skip to content

Move credential details to vendor configuration - #3085

Merged
gardener-prow[bot] merged 2 commits into
masterfrom
enh/vendor-secret-details
Jul 23, 2026
Merged

Move credential details to vendor configuration#3085
gardener-prow[bot] merged 2 commits into
masterfrom
enh/vendor-secret-details

Conversation

@grolu

@grolu grolu commented Jul 17, 2026

Copy link
Copy Markdown
Member

What this PR does / why we need it:

This is the third preparatory refactor extracted from #2824 and follows the vendor registry extraction in #3075 and typed vendor lookups in #3082.

Credential detail rendering previously used a provider-name switch in credential/helper.js. That kept provider-specific display metadata in application code and required extending the switch for every supported vendor.

This change:

  • moves read-only credential-detail definitions into each infrastructure and DNS vendor module under frontend/src/data/vendors
  • replaces the provider switch with a configuration-driven detail resolver
  • supports the existing Azure, vSphere, and RFC2136 Secret-key aliases
  • safely resolves GCP project IDs from structured service-account data, including the existing Google Cloud DNS fallback
  • looks up vendor configuration through the explicit infra or dns domain
  • renames the DNS-only raw-credential composable to useDnsProviderCredential

Related PRs:

Which issue(s) this PR fixes:

None.

Special notes for your reviewer:

Please review this as the third preparatory refactor in the sequential split of #2824. The secret.details configuration is read-only display metadata; it is not used when creating or updating Secrets.

Validation:

  • make verify
  • focused credential-helper, infrastructure-binding, and DNS-credential composable tests
  • provider-by-provider snapshot coverage for credential detail output
  • GitGuardian pre-commit scan

Release note:

NONE

Summary by CodeRabbit

  • New Features

    • Added vendor-aware credential details for infrastructure and DNS secrets, including project fields derived from service account content.
    • Improved rendering by passing explicit vendor type into the credential details display.
    • Added support for hidden credential values (e.g., API keys and tokens) in vendor definitions.
  • Bug Fixes

    • Credential details now safely handle missing/unknown provider data, optional secrets, and malformed or unsupported secret-value mappings.
  • Tests

    • Expanded Jest coverage for DNS/infra credential resolution, hidden details behavior, and invalid configuration edge cases.

@gardener-prow

gardener-prow Bot commented Jul 17, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@gardener-prow gardener-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Credential secret details are now declared in infrastructure and DNS vendor configurations. Shared resolution supports alternate keys, Base64 decoding, JSON parsing, nested paths, and hidden fields. Credential composables and components pass vendor-specific configurations into the resolver, with expanded tests.

Changes

Vendor-configured secret details

Layer / File(s) Summary
Vendor metadata and secret detail resolver
frontend/src/composables/credential/helper.js, frontend/src/data/vendors/dns/*, frontend/src/data/vendors/infra/*
Vendor definitions declare secret detail fields, and secretDetails resolves values from those declarations.
Provider configuration integration
frontend/src/composables/credential/useCloudProviderBinding.js, frontend/src/composables/credential/useDnsProviderCredential.js
Infrastructure and DNS composables obtain providerConfig through configStore.vendorDetails before resolving secret details.
Credential component wiring
frontend/src/components/Credentials/*, frontend/src/components/ShootDetails/*, frontend/src/components/ShootDns/*, frontend/src/views/GCredentials.vue
Credential detail components receive explicit vendor types, while DNS credential paths use useDnsProviderCredential.
Credential detail validation
frontend/__tests__/composables/*
Tests cover configured detail mappings, resolver edge cases, missing configurations, undefined secrets, DNS credential behavior, and invalid service-account JSON.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CredentialView
  participant CredentialComposable
  participant ConfigStore
  participant SecretDetails
  CredentialView->>CredentialComposable: resolve credential details
  CredentialComposable->>ConfigStore: vendorDetails({ type, name })
  ConfigStore-->>CredentialComposable: return providerConfig
  CredentialComposable->>SecretDetails: resolve secret with providerConfig
  SecretDetails-->>CredentialView: return labeled values
Loading

Possibly related PRs

Suggested labels: area/quality, approved, lgtm

Suggested reviewers: holgerkoser, klocke-io

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: moving credential details into vendor configuration.
Description check ✅ Passed Core template sections are filled, but the /area and /kind categorization fields from the template are still missing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch enh/vendor-secret-details

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gardener-prow gardener-prow Bot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 17, 2026
@grolu
grolu marked this pull request as ready for review July 17, 2026 14:55
@grolu
grolu requested a review from a team as a code owner July 17, 2026 14:55
@gardener-prow gardener-prow Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2026
@grolu grolu added kind/enhancement Enhancement, improvement, extension do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 17, 2026
@gardener-prow gardener-prow Bot removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 17, 2026
@gardener-prow gardener-prow Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 23, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/data/vendors/dns/google-clouddns.js`:
- Around line 10-13: The Google Cloud DNS valueFrom definition must support both
serviceaccount.json credentials and raw project_id credentials. Update the
relevant resolver configuration or implementation so each ordered alternative
uses the correct parsing and path behavior, rather than applying JSON parsing to
the raw value; retain serviceaccount.json as the primary option and add coverage
for raw project_id resolution.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 456adfba-e669-41cf-b5b8-c6dfd105a9b9

📥 Commits

Reviewing files that changed from the base of the PR and between e0badf5 and cf91edb.

⛔ Files ignored due to path filters (1)
  • frontend/__tests__/composables/__snapshots__/credential.helper.spec.js.snap is excluded by !**/*.snap
📒 Files selected for processing (18)
  • frontend/__tests__/composables/credential.helper.spec.js
  • frontend/__tests__/composables/useCloudProviderBinding.spec.js
  • frontend/__tests__/composables/useDnsProviderCredential.spec.js
  • frontend/src/composables/credential/helper.js
  • frontend/src/data/vendors/dns/aws-route53.js
  • frontend/src/data/vendors/dns/azure-dns.js
  • frontend/src/data/vendors/dns/google-clouddns.js
  • frontend/src/data/vendors/dns/infoblox.js
  • frontend/src/data/vendors/dns/openstack-designate.js
  • frontend/src/data/vendors/dns/powerdns.js
  • frontend/src/data/vendors/dns/rfc2136.js
  • frontend/src/data/vendors/infra/alicloud.js
  • frontend/src/data/vendors/infra/aws.js
  • frontend/src/data/vendors/infra/azure.js
  • frontend/src/data/vendors/infra/gcp.js
  • frontend/src/data/vendors/infra/metal.js
  • frontend/src/data/vendors/infra/openstack.js
  • frontend/src/data/vendors/infra/vsphere.js
🚧 Files skipped from review as they are similar to previous changes (15)
  • frontend/src/data/vendors/infra/vsphere.js
  • frontend/src/data/vendors/dns/powerdns.js
  • frontend/src/data/vendors/infra/alicloud.js
  • frontend/src/data/vendors/dns/openstack-designate.js
  • frontend/src/data/vendors/infra/openstack.js
  • frontend/src/data/vendors/dns/aws-route53.js
  • frontend/src/data/vendors/dns/infoblox.js
  • frontend/src/data/vendors/infra/aws.js
  • frontend/src/data/vendors/dns/rfc2136.js
  • frontend/src/data/vendors/dns/azure-dns.js
  • frontend/src/data/vendors/infra/gcp.js
  • frontend/tests/composables/useCloudProviderBinding.spec.js
  • frontend/tests/composables/useDnsProviderCredential.spec.js
  • frontend/src/data/vendors/infra/metal.js
  • frontend/src/composables/credential/helper.js

Comment thread frontend/src/data/vendors/dns/google-clouddns.js

@petersutter petersutter 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.

/lgtm
/approve

@gardener-prow gardener-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2026
@gardener-prow

gardener-prow Bot commented Jul 23, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petersutter

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow

gardener-prow Bot commented Jul 23, 2026

Copy link
Copy Markdown

LGTM label has been added.

DetailsGit tree hash: 9b32a199deafa1485798a78f98e6bec64c065c41

@gardener-prow gardener-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 23, 2026
@gardener-prow
gardener-prow Bot merged commit 930cf32 into master Jul 23, 2026
38 checks passed
@gardener-prow
gardener-prow Bot deleted the enh/vendor-secret-details branch July 23, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants