Skip to content

fix: Add test case deploying an Orchestrator-flavored instance with a user provided npmrc secret [RHDHBUGS-1893] - #195

Merged
rm3l merged 4 commits into
redhat-developer:mainfrom
rm3l:rhdhbugs-1893-tech-debt-helm-add-test-case-deploying-orchestrator-flavored-rhdh-with-user-provided-npmrc-secret-to-prevent-future-regression-on-this-scenario
Jul 21, 2025
Merged

fix: Add test case deploying an Orchestrator-flavored instance with a user provided npmrc secret [RHDHBUGS-1893]#195
rm3l merged 4 commits into
redhat-developer:mainfrom
rm3l:rhdhbugs-1893-tech-debt-helm-add-test-case-deploying-orchestrator-flavored-rhdh-with-user-provided-npmrc-secret-to-prevent-future-regression-on-this-scenario

Conversation

@rm3l

@rm3l rm3l commented Jul 18, 2025

Copy link
Copy Markdown
Member

Description of the change

This is a follow-up to #186. It tries to prevent future regression on this by testing this scenario.

Which issue(s) does this PR fix or relate to

How to test changes / Special notes to the reviewer

Without the changes in #186, deploying with the charts/backstage/ci/with-orchestrator-and-dynamic-plugins-npmrc-values.yaml values file fails (as expected) with the following error:

Error: 1 error occurred:
        * secrets "my-backstage-orch-1-dynamic-plugins-npmrc" already exists

With the changes in #186, it passes.

Checklist

  • For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
  • For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Use pre-commit run -a to apply changes. The pre-commit Workflow will do this automatically for you if needed.
  • JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
  • Tests pass using the Chart Testing tool and the ct lint command.
  • If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

Summary by Sourcery

Enable injection of a test-only dynamic plugins npmrc Secret in Helm tests to fix secret conflict regressions, bump the chart version, update documentation, and add an example values file for orchestrator scenarios.

New Features:

  • Add test.injectTestNpmrcSecret flag to helm values allowing injection of a dynamic plugins npmrc Secret via pre-install/pre-upgrade hooks
  • Introduce a test-secret template to create a dummy npmrc Secret when the injection is enabled

Bug Fixes:

  • Prevent test failures caused by existing dynamic plugins npmrc secrets when deploying an orchestrator-flavored instance

Enhancements:

  • Provide a sample CI values file for running Backstage in orchestrator mode with dynamic plugins and npmrc secret injection

Build:

  • Bump charts/backstage version from 4.5.0 to 4.5.1

Documentation:

  • Update values.yaml and README to document the new injectTestNpmrcSecret option

Summary by Sourcery

Enable test-only injection of a dynamic plugins npmrc Secret to prevent Helm test failures when deploying orchestrator-flavored Backstage instances under user-provided npmrc scenarios, bump the chart version, document the new option, and supply an example values file.

New Features:

  • Add test.injectTestNpmrcSecret flag to Helm values to enable injection of a dummy dynamic plugins npmrc Secret via pre-install/pre-upgrade hooks

Bug Fixes:

  • Prevent secret conflict errors by injecting a test-only npmrc Secret when running Helm tests with orchestrator and dynamic plugins

Enhancements:

  • Provide a sample CI values file for orchestrator mode with dynamic plugins and npmrc secret injection

Build:

  • Bump charts/backstage version from 4.5.0 to 4.5.1

Documentation:

  • Document the injectTestNpmrcSecret option in values.yaml and README

Tests:

  • Add a test-secret Helm template to create a dummy npmrc Secret during tests

@sourcery-ai

sourcery-ai Bot commented Jul 18, 2025

Copy link
Copy Markdown

Reviewer's Guide

Introduce a test-only dynamic plugins npmrc secret injection in Helm tests to prevent secret conflicts when deploying an Orchestrator-flavored Backstage instance, bump the chart version to 4.5.1, update documentation, and include a sample CI values file for this scenario.

Entity relationship diagram for test-only npmrc Secret injection

erDiagram
    TEST_CONFIG ||--o{ TEST_SECRET : injects
    TEST_SECRET {
        bool injectTestNpmrcSecret
    }
    TEST_CONFIG {
        bool enabled
        string imageRegistry
        string imageRepository
        string imageTag
        bool injectTestNpmrcSecret
    }
Loading

Class diagram for the new test.injectTestNpmrcSecret Helm value

classDiagram
    class TestConfig {
        +bool enabled
        +ImageConfig image
        +bool injectTestNpmrcSecret
    }
    class ImageConfig {
        +string registry
        +string repository
        +string tag
    }
    TestConfig --> ImageConfig
Loading

File-Level Changes

Change Details Files
Add injectTestNpmrcSecret flag for test-secret injection
  • Added injectTestNpmrcSecret field in test values section of values.yaml with default false
  • Documented the new flag and its behavior in values.yaml comments
  • Updated README.md values table to include test.injectTestNpmrcSecret entry
charts/backstage/values.yaml
charts/backstage/README.md
Introduce test-secret template to generate dummy npmrc Secret
  • Created tests/test-secret.yaml to conditionally render a Secret when test.injectTestNpmrcSecret is true
  • Added Helm hooks (pre-install, pre-upgrade) and weight annotation to the Secret template
charts/backstage/templates/tests/test-secret.yaml
Provide CI values file for orchestrator with dynamic-plugins npmrc injection
  • Added with-orchestrator-and-dynamic-plugins-npmrc-values.yaml sample showing orchestrator enabled and injectTestNpmrcSecret set to true
  • Configured dynamic plugins and disabled persistence for CI scenarios
charts/backstage/ci/with-orchestrator-and-dynamic-plugins-npmrc-values.yaml
Bump chart version and update documentation badge
  • Updated Chart.yaml version from 4.5.0 to 4.5.1
  • Updated README.md shield badge to reflect new version
charts/backstage/Chart.yaml
charts/backstage/README.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@rm3l

rm3l commented Jul 18, 2025

Copy link
Copy Markdown
Member Author

/cherry-pick release-1.7

@openshift-cherrypick-robot

Copy link
Copy Markdown

@rm3l: once the present PR merges, I will cherry-pick it on top of release-1.7 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@rm3l
rm3l force-pushed the rhdhbugs-1893-tech-debt-helm-add-test-case-deploying-orchestrator-flavored-rhdh-with-user-provided-npmrc-secret-to-prevent-future-regression-on-this-scenario branch from 3e8f819 to ebda649 Compare July 18, 2025 17:09
@rm3l
rm3l force-pushed the rhdhbugs-1893-tech-debt-helm-add-test-case-deploying-orchestrator-flavored-rhdh-with-user-provided-npmrc-secret-to-prevent-future-regression-on-this-scenario branch from ebda649 to f7a0a73 Compare July 18, 2025 19:56
@rm3l rm3l changed the title fix: Add test case deploying an Orchestrator-flavored instance with a user provided npmrc secret to prevent future regression on this scenario fix: Add test case deploying an Orchestrator-flavored instance with a user provided npmrc secret to prevent future regression on this scenario [RHDHBUGS-1893] Jul 18, 2025
@rm3l rm3l changed the title fix: Add test case deploying an Orchestrator-flavored instance with a user provided npmrc secret to prevent future regression on this scenario [RHDHBUGS-1893] fix: Add test case deploying an Orchestrator-flavored instance with a user provided npmrc secret [RHDHBUGS-1893] Jul 18, 2025
@rm3l
rm3l marked this pull request as ready for review July 21, 2025 10:08

@sourcery-ai sourcery-ai 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.

Hey @rm3l - I've reviewed your changes - here's some feedback:

  • Consider adding a helm.sh/hook-delete-policy annotation to the test-secret hook so it cleans up between runs and avoids secret conflicts on upgrades.
  • Add a reference to the new CI example values file (with-orchestrator-and-dynamic-plugins-npmrc-values.yaml) in the chart README so users can easily find and use it.
  • Verify that the chart's JSON schema has been updated to include the new test.injectTestNpmrcSecret field for proper validation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a helm.sh/hook-delete-policy annotation to the test-secret hook so it cleans up between runs and avoids secret conflicts on upgrades.
- Add a reference to the new CI example values file (with-orchestrator-and-dynamic-plugins-npmrc-values.yaml) in the chart README so users can easily find and use it.
- Verify that the chart's JSON schema has been updated to include the new test.injectTestNpmrcSecret field for proper validation.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

rm3l added 3 commits July 21, 2025 13:26
… pre-install/pre-upgrade hook

This is to simulate a user creating their external npmrc Secret

Doing it this way because the secret name is dynamic and depends on the release name
…ew test.injectTestNpmrcSecret field for proper validation
@rm3l
rm3l force-pushed the rhdhbugs-1893-tech-debt-helm-add-test-case-deploying-orchestrator-flavored-rhdh-with-user-provided-npmrc-secret-to-prevent-future-regression-on-this-scenario branch from f7a0a73 to 34dd5d8 Compare July 21, 2025 11:47
@rm3l

rm3l commented Jul 21, 2025

Copy link
Copy Markdown
Member Author
  • Consider adding a helm.sh/hook-delete-policy annotation to the test-secret hook so it cleans up between runs and avoids secret conflicts on upgrades.

No need. If no hook deletion policy annotation is specified, the before-hook-creation behavior applies by default, which is fine for this case.

  • Add a reference to the new CI example values file (with-orchestrator-and-dynamic-plugins-npmrc-values.yaml) in the chart README so users can easily find and use it.

Not needed. All CI values are located under a standard directory.

  • Verify that the chart's JSON schema has been updated to include the new test.injectTestNpmrcSecret field for proper validation.

Done in 34dd5d8 (#195)

Co-authored-by: rm3l <rm3l@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

⚠️ Files changed after running the pre-commit hooks

Those changes should have been pushed automatically to your PR branch.

NOTE: If the PR checks are stuck after this additional commit, manually close the PR and immediately reopen it to trigger the checks again.

@rm3l rm3l closed this Jul 21, 2025
@rm3l rm3l reopened this Jul 21, 2025
@sonarqubecloud

Copy link
Copy Markdown

@rm3l
rm3l merged commit e42438b into redhat-developer:main Jul 21, 2025
7 of 8 checks passed
@rm3l
rm3l deleted the rhdhbugs-1893-tech-debt-helm-add-test-case-deploying-orchestrator-flavored-rhdh-with-user-provided-npmrc-secret-to-prevent-future-regression-on-this-scenario branch July 21, 2025 14:35
@openshift-cherrypick-robot

Copy link
Copy Markdown

@rm3l: #195 failed to apply on top of branch "release-1.7":

Applying: fix: Add an option to inject a test dynamic plugins npmrc Secret as a pre-install/pre-upgrade hook
Applying: Bump chart version and regenerate README
Using index info to reconstruct a base tree...
M	charts/backstage/Chart.yaml
M	charts/backstage/README.md
Falling back to patching base and 3-way merge...
Auto-merging charts/backstage/README.md
CONFLICT (content): Merge conflict in charts/backstage/README.md
Auto-merging charts/backstage/Chart.yaml
CONFLICT (content): Merge conflict in charts/backstage/Chart.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0002 Bump chart version and regenerate README

Details

In response to this:

/cherry-pick release-1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

rm3l added a commit to rm3l/rhdh-chart that referenced this pull request Jul 21, 2025
… user provided npmrc secret [RHDHBUGS-1893] (redhat-developer#195)

Co-authored-by: rm3l <rm3l@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
rm3l added a commit that referenced this pull request Jul 21, 2025
… user provided npmrc secret [RHDHBUGS-1893] (#195) (#197)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

3 participants