Skip to content

fix: harden git secret flow and preserve category label on update#548

Merged
JanakaSandaruwan merged 6 commits into
openchoreo:mainfrom
JanakaSandaruwan:feat/migrate-git-secrets-v2
May 15, 2026
Merged

fix: harden git secret flow and preserve category label on update#548
JanakaSandaruwan merged 6 commits into
openchoreo:mainfrom
JanakaSandaruwan:feat/migrate-git-secrets-v2

Conversation

@JanakaSandaruwan
Copy link
Copy Markdown
Contributor

@JanakaSandaruwan JanakaSandaruwan commented May 15, 2026

Purpose

Follow-up to #541. Two issues surfaced after that PR landed:

  • The scaffolder git-secret picker called GET /secrets even when the
    secretManagement feature flag was off, which returned a 500 and showed
    a user-facing error on a form they couldn't act on. The "Create New Git
    Secret" option was also enabled in that state.
  • EditSecretDialog did not send labels on update. The backend replaces
    the full user-set label map on each PUT, so editing a git-credentials
    secret silently stripped its openchoreo.dev/secret-type label — after
    which it no longer appeared in the scaffolder/CI git-secret dropdowns.

Approach

  • GitSourceField reads useSecretManagementEnabled and short-circuits the
    /secrets fetch when the flag is off, so no more spurious 500 errors.
    "Create New Git Secret" is now disabled with a tooltip when the flag is
    off or no workflow is selected; the tooltip anchors its left edge to the
    option's left edge.
  • Removed the standalone GitSecretField scaffolder extension. The CTD
    converter never emitted ui:field: GitSecretField, so it was dead code.
    GitSecretDialog stays because GitSourceField still uses it.
  • CreateSecretDialog now stamps the category label on every new secret —
    openchoreo.dev/secret-type: generic for the default Generic category
    and git-credentials for Git Credentials — so the filter logic is
    symmetric.
  • SecretsService.getSecret returns the SecretReference's labels alongside
    the data, and EditSecretDialog echoes those labels back on PUT so
    existing category labels survive value edits. Added a regression test.
Screenshot 2026-05-15 at 14 39 24

Related Issues

N/A

Checklist

  • Tests added or updated (unit, integration, etc.)
  • Samples updated (if applicable)

Signed-off-by: Janaka Sandaruwan <janakasandaruwan1996@gmail.com>
Signed-off-by: Janaka Sandaruwan <janakasandaruwan1996@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Warning

Rate limit exceeded

@JanakaSandaruwan has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 8 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f3fba318-80da-4eca-b948-15bac253b72f

📥 Commits

Reviewing files that changed from the base of the PR and between b26784e and 6ac7133.

📒 Files selected for processing (4)
  • plugins/openchoreo-backend/src/services/SecretsService/SecretsService.test.ts
  • plugins/openchoreo-common/src/constants.test.ts
  • plugins/openchoreo-react/src/components/NamespaceScopeFilter/NamespaceScopeFilter.tsx
  • plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx
📝 Walkthrough

Walkthrough

This PR removes the deprecated GitSecretField scaffolder, integrates a secret management feature flag into GitSourceField to control secret operations, establishes backend support for secret category labels, and ensures secrets are consistently labeled during creation and their labels are preserved during edits.

Changes

Secret Management Refactor

Layer / File(s) Summary
Deprecate GitSecretField scaffolder component
packages/app/src/App.tsx, packages/app/src/scaffolder/GitSecretField/*
GitSecretField component, schema, validation, and extensions are entirely removed. App.tsx swaps the import from GitSecretFieldExtension to GitSourceFieldExtension and removes GitSecretFieldExtension from the ScaffolderFieldExtensions list.
Add secret management feature flag to GitSourceField
packages/app/src/scaffolder/GitSourceField/GitSourceField.tsx
Integrate useSecretManagementEnabled to gate secret fetching and creation. Secret fetching skips API calls when disabled; "Create New Git Secret" option becomes non-interactive with a tooltip when flag is off or workflow plane annotations are absent. Autocomplete uses getOptionDisabled to disable divider and conditional create option.
Backend secret detail label support
plugins/openchoreo-backend/src/services/SecretsService/SecretsService.ts
SecretDetail interface gains optional labels field sourced from SecretReference metadata. The getSecret method populates this field from ref.metadata?.labels.
Add generic secret type constant
plugins/openchoreo-common/src/constants.ts, plugins/openchoreo-common/src/index.ts
Define GENERIC_SECRET_TYPE_VALUE as 'generic' to label secrets created via generic Secret API. Export from common package index for use across dialogs.
Stamp secret category labels on creation
plugins/openchoreo/src/components/Secrets/CreateSecretDialog.tsx, plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx
CreateSecretDialog now always sets CHOREO_LABELS.SECRET_TYPE label based on secretCategory (GIT_SECRET_TYPE_VALUE for git-credentials, GENERIC_SECRET_TYPE_VALUE otherwise). Tests verify generic label is stamped and SSH payloads include expected fields with extended timeouts.
Preserve secret labels through edit flow
plugins/openchoreo/src/components/Secrets/EditSecretDialog.tsx, plugins/openchoreo/src/components/Secrets/EditSecretDialog.test.tsx
EditSecretDialog tracks existingLabels state, seeds it during dialog open, refines it after detail fetch, and includes both data and labels in submit payload. Test verifies original labels are preserved across edit operations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • openchoreo/backstage-plugins#536: Introduces the secretManagement feature flag that gates secret operations and implements upstream secret flows via CreateSecretDialog/EditSecretDialog with label handling.
  • openchoreo/backstage-plugins#541: Complements label-based secret categorization by migrating generic secret operations and updating SecretsService label plumbing for category handling.

Suggested reviewers

  • kaviththiranga
  • sameerajayasoma
  • stefinie123

🐰 A secret field departs with grace,
While labels now stamp every case,
Feature flags guide what's permitted,
And edits keep labels committed—
Security categories find their place!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description addresses the purpose and approach thoroughly, including specific technical details and a screenshot. However, it lacks several required template sections such as Goals, User stories, Release note, Documentation, Training, Certification, Marketing, and test environment details. Complete the PR description by adding: explicit Goals section, User stories, Release note summary, Documentation impact assessment, Training/Certification/Marketing sections (or N/A with explanation), and test environment details.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: hardening the git secret flow by fixing the feature flag check and preserving category labels during edits.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx (1)

123-139: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add test coverage for git-credentials category.

The test suite verifies generic category label stamping but lacks coverage for the git-credentials path. Since the PR objectives specifically address git-credentials secret filtering and the dialog code (CreateSecretDialog.tsx lines 418-421) branches on secretCategory, both paths should be tested.

🧪 Proposed test for git-credentials category
   });
+
+  it('stamps the git-credentials label when the category is Git Credentials', async () => {
+    const user = userEvent.setup();
+    const onSubmit = jest.fn().mockResolvedValue({} as any);
+    renderDialog({ targetPlanes: planes, onSubmit });
+
+    await user.type(inputForLabel('Secret Name'), 'git-secret');
+    await user.click(screen.getByRole('button', { name: /Secret Category/i }));
+    await user.click(screen.getByRole('option', { name: /Git Credentials/i }));
+    await user.click(screen.getByRole('radio', { name: /Basic Auth/i }));
+    await user.type(inputForLabel('Password / Token'), 'ghp_token');
+    await user.click(screen.getByRole('button', { name: 'Create' }));
+
+    expect(onSubmit).toHaveBeenCalledTimes(1);
+    expect(onSubmit.mock.calls[0][0].labels).toEqual({
+      'openchoreo.dev/secret-type': 'git-credentials',
+    });
+  });

 });
🤖 Prompt for 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.

In `@plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx` around
lines 123 - 139, Add a new test in CreateSecretDialog.test.tsx mirroring the
existing "stamps the generic label" test but exercising the git-credentials
branch: use renderDialog (same helper), simulate user typing the relevant fields
via inputForLabel (e.g., "Secret Name", "Username", "Password / Token"), select
the radio labeled "Git Credentials" (or the exact option text used in the
dialog), click the Create button, and assert that onSubmit was called once and
that onSubmit.mock.calls[0][0].labels equals { 'openchoreo.dev/secret-type':
'git-credentials' }; this ensures the CreateSecretDialog secretCategory branch
(CreateSecretDialog.tsx handling for git-credentials) is covered.
🤖 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.

Outside diff comments:
In `@plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx`:
- Around line 123-139: Add a new test in CreateSecretDialog.test.tsx mirroring
the existing "stamps the generic label" test but exercising the git-credentials
branch: use renderDialog (same helper), simulate user typing the relevant fields
via inputForLabel (e.g., "Secret Name", "Username", "Password / Token"), select
the radio labeled "Git Credentials" (or the exact option text used in the
dialog), click the Create button, and assert that onSubmit was called once and
that onSubmit.mock.calls[0][0].labels equals { 'openchoreo.dev/secret-type':
'git-credentials' }; this ensures the CreateSecretDialog secretCategory branch
(CreateSecretDialog.tsx handling for git-credentials) is covered.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 66e33ae1-7579-4db4-be35-470ab1099ae5

📥 Commits

Reviewing files that changed from the base of the PR and between ea8d8b8 and b26784e.

📒 Files selected for processing (12)
  • packages/app/src/App.tsx
  • packages/app/src/scaffolder/GitSecretField/GitSecretField.tsx
  • packages/app/src/scaffolder/GitSecretField/extensions.ts
  • packages/app/src/scaffolder/GitSecretField/index.ts
  • packages/app/src/scaffolder/GitSourceField/GitSourceField.tsx
  • plugins/openchoreo-backend/src/services/SecretsService/SecretsService.ts
  • plugins/openchoreo-common/src/constants.ts
  • plugins/openchoreo-common/src/index.ts
  • plugins/openchoreo/src/components/Secrets/CreateSecretDialog.test.tsx
  • plugins/openchoreo/src/components/Secrets/CreateSecretDialog.tsx
  • plugins/openchoreo/src/components/Secrets/EditSecretDialog.test.tsx
  • plugins/openchoreo/src/components/Secrets/EditSecretDialog.tsx
💤 Files with no reviewable changes (4)
  • packages/app/src/scaffolder/GitSecretField/GitSecretField.tsx
  • packages/app/src/scaffolder/GitSecretField/index.ts
  • packages/app/src/scaffolder/GitSecretField/extensions.ts
  • packages/app/src/App.tsx

@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

❌ Patch coverage is 40.00000% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...p/src/scaffolder/GitSourceField/GitSourceField.tsx 9.09% 8 Missing and 2 partials ⚠️
...ents/NamespaceScopeFilter/NamespaceScopeFilter.tsx 0.00% 1 Missing ⚠️
...oreo/src/components/Secrets/CreateSecretDialog.tsx 66.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Signed-off-by: Janaka Sandaruwan <janakasandaruwan1996@gmail.com>
Signed-off-by: Janaka Sandaruwan <janakasandaruwan1996@gmail.com>
Signed-off-by: Janaka Sandaruwan <janakasandaruwan1996@gmail.com>
Signed-off-by: Janaka Sandaruwan <janakasandaruwan1996@gmail.com>
@JanakaSandaruwan JanakaSandaruwan merged commit f2eca8d into openchoreo:main May 15, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants