Skip to content

fix(harbor): mark credentials Secret with user-secret label - #2541

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
fix/harbor-user-secret-credentials
Apr 30, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
fix/harbor-user-secret-credentials

Conversation

@IvanHunters

@IvanHunters IvanHunters commented Apr 29, 2026 •

Copy link
Copy Markdown
Collaborator

What this PR does

  • Stamps the Helm-managed {{ .Release.Name }}-credentials Secret with apps.cozystack.io/user-secret: "true".
  • Adds a matchLabels rule to secrets.include in the Harbor ApplicationDefinition, alongside the existing resourceNames entry.

The lineage webhook decides whether a Secret is tenant-visible by matching its name against the secrets.include[].resourceNames template in the corresponding ApplicationDefinition. For Harbor the template {{ .name }}-credentials renders to {name}-credentials and does not account for the release.prefix: "harbor-" set in the same definition, while the Helm chart actually creates harbor-{name}-credentials. The names never match, so the Secret ends up labelled internal.cozystack.io/tenantresource=false and never appears in the TenantSecrets API.

This change follows the existing apps.cozystack.io/user-secret: "true" pattern already used by Bucket (packages/system/bucket/templates/user-credentials.yaml) and RabbitMQ (packages/apps/rabbitmq/templates/rabbitmq.yaml). The webhook then matches via labels and is independent of how Helm names the Secret.

Screenshots

Not applicable — no UI changes in this PR.

Release note

fix(harbor): expose Harbor credentials Secret to tenant-aware clients by marking it with the `apps.cozystack.io/user-secret` label, mirroring the pattern already used by Bucket and RabbitMQ.

Summary by CodeRabbit

  • Chores
    • Updated Harbor configuration to support label-based secret discovery alongside existing name-based selection, improving secret management reliability.

The Helm-managed harbor-{name}-credentials Secret was not visible
through the TenantSecrets API because the lineage webhook compared the
real Secret name against the rendered template "{{ .name }}-credentials"
(which becomes "{name}-credentials" without the "harbor-" HelmRelease
prefix), failed to match, and labelled the Secret as
internal.cozystack.io/tenantresource=false.

Adopt the same pattern Bucket and RabbitMQ already use: stamp the
user-facing Secret with apps.cozystack.io/user-secret=true and add a
matchLabels rule alongside the existing resourceNames entry in the
ApplicationDefinition. The webhook now matches via labels instead of
relying on Secret-name templating, so the Secret is properly marked
tenantresource=true and surfaces in tenant-aware clients.

Signed-off-by: IvanHunters <[email protected]>
@coderabbitai

coderabbitai Bot commented Apr 29, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fb2d8e41-c30e-468a-82b0-9110e182275e

📥 Commits

Reviewing files that changed from the base of the PR and between f45facc and fdbb6fd.

📒 Files selected for processing (2)
  • packages/apps/harbor/templates/harbor.yaml
  • packages/system/harbor-rd/cozyrds/harbor.yaml

📝 Walkthrough

Walkthrough

The pull request adds label-based secret discovery to Harbor's configuration. A Kubernetes label is added to credentials secrets, and the Harbor ApplicationDefinition is updated to discover secrets using this label alongside existing name-based matching.

Changes

Cohort / File(s) Summary
Label-based Secret Discovery
packages/apps/harbor/templates/harbor.yaml, packages/system/harbor-rd/cozyrds/harbor.yaml
Adds Kubernetes label apps.cozystack.io/user-secret: "true" to credentials secret metadata, and extends ApplicationDefinition secret selection logic to discover secrets by label in addition to name-based matching.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 With labels bright, the secrets sing,
No more just names to guide the thing!
A fuzzy discovery unfolds so neat,
Harbor's credentials now hide and greet,
Through metadata magic, complete and sweet! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main change: adding a user-secret label to the Harbor credentials Secret, which is the core fix addressing the tenant-visibility issue.
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
  • Commit unit tests in branch fix/harbor-user-secret-credentials

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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

@github-actions github-actions Bot added area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/bug Categorizes issue or PR as related to a bug labels Apr 29, 2026
@IvanHunters
IvanHunters marked this pull request as ready for review April 29, 2026 17:49
@dosubot dosubot Bot added the size/XS This PR changes 0-9 lines, ignoring generated files label Apr 29, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where Harbor credentials were not appearing in the TenantSecrets API because the lineage webhook failed to match the Secret name due to Helm naming conventions. By adopting the standard labeling pattern used by other system components, the webhook can now reliably identify and expose these credentials to tenant-aware clients.

Highlights

  • Secret Labeling: Added the 'apps.cozystack.io/user-secret: "true"' label to the Harbor credentials Secret to ensure proper identification by the lineage webhook.
  • ApplicationDefinition Update: Updated the Harbor ApplicationDefinition to include a matchLabels rule for the user-secret label, resolving issues where the Secret was incorrectly excluded from the TenantSecrets API due to naming mismatches.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment Gemini (@gemini-code-assist) Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a label-based approach for identifying Harbor credential secrets by adding the apps.cozystack.io/user-secret: "true" label to the secret and updating the Harbor RDS configuration to match it. Feedback suggests removing the redundant and broken resourceNames entry in the RDS configuration to improve maintainability and avoid confusion.

Comment on lines +36 to +37
- matchLabels:
apps.cozystack.io/user-secret: "true"

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.

low

The addition of matchLabels correctly addresses the visibility issue for the credentials secret. However, as noted in the PR description, the existing resourceNames entry (line 35) is currently broken due to the harbor- prefix mismatch. Since the label-based approach is more robust and aligns with the pattern used in other packages like Bucket and RabbitMQ, the redundant resourceNames entry could be removed in a future cleanup to keep the configuration clean and avoid confusion.

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.

APPROVE — fixes the stated bug correctly. The matchLabels approach makes the credentials Secret discoverable independently of Helm's release-name prefix and matches the Bucket/RabbitMQ pattern.

Three non-blocking follow-ups for separate PRs (do not gate this one):

  1. The existing resourceNames: ["{{ .name }}-credentials"] entry in harbor-rd/cozyrds/harbor.yaml is now dead code — release.prefix: "harbor-" makes the chart-managed Secret harbor-{name}-credentials, which never matches that template. Bucket and RabbitMQ include the prefix in their resourceNames (bucket-{{ .name }}-credentials, rabbitmq-{{ .name }}-default-user) and pair it with matchLabels for defense in depth. Either fix this line to harbor-{{ .name }}-credentials to mirror that, or drop the dead entry.

  2. The same prefix mismatch likely affects services.include ({{ .name }}) and ingresses.include ({{ .name }}-ingress) in the same file. The Helm chart sets harbor.fullnameOverride: {{ .Release.Name }} so the rendered names are harbor-{name}*. If the lineage webhook is not prefix-aware for those resource kinds either, Harbor's TenantServices and TenantIngresses are silently broken too. Worth a follow-up.

  3. Pre-existing docs gap (predates this PR, do not gate): the Harbor docs page does not document how to retrieve the admin password. Now that this PR makes the credentials Secret reachable via the TenantSecrets API, the docs can finally describe that workflow — tracked in cozystack/website#522.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Apr 29, 2026
@lexfrei
Aleksei Sviridkin (lexfrei) merged commit 4dbd4b0 into main Apr 30, 2026
14 checks passed
@lexfrei
Aleksei Sviridkin (lexfrei) deleted the fix/harbor-user-secret-credentials branch April 30, 2026 09:40
Timofei Larkin (lllamnyp) added a commit that referenced this pull request Sep 14, 2026
…4170)

<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `kind/backport` if it's a bugfix that needs to be
backported to a previous version.
-->

## What this PR does

`harbor` declares `release.prefix: "harbor-"` and then selects its
Secret, Service and Ingress without it, so all three `resourceNames`
selectors match nothing. This corrects the three names and adds a guard
so the next omission fails a test instead of passing silently.

### Why they never matched

Release names are `<prefix><app name>`, but the lineage webhook
evaluates selectors against `{kind, name, namespace}`, where `name` is
the custom resource's name and carries no prefix. Every definition
therefore spells the prefix out itself, and the others do — `postgres-{{
.name }}-credentials`, `bucket-{{ .name }}-credentials`, and so on.
`harbor` is the only one that does not.

A selector that matches nothing raises no error: the webhook labels the
object `internal.cozystack.io/tenantresource=false` and continues, which
is indistinguishable from a selector that legitimately has nothing to
match. That is why this survived.

### What actually changes at runtime

Close to nothing, and that is the point — the manifest was wrong, not
the behaviour:

- the Secret already reaches tenants, because #2541 worked around this
exact mismatch by stamping `apps.cozystack.io/user-secret: "true"` on it
and matching by label. That workaround is deliberately left in place;
- the Service and Ingress labels flip from `false` to `true`, which
nothing consumes: `spec.services` and `spec.ingresses` are read only by
the webhook that computes the label, and the label's only consumer is
the TenantSecret registry, which handles Secrets. The dashboard reaches
both through the per-release Role, which names them correctly.

So this closes a latent defect before it bites, rather than fixing an
outage.

### The guard

`hack/check-rd-release-prefix.bats` asserts that every `resourceNames`
entry in `packages/system/*-rd/cozyrds/*.yaml` contains `<prefix>{{
.name }}`. `hack/*.bats` is auto-discovered by `BATS_UNIT_FILES`, so it
runs under `make unit-tests` with no registration.

The assertion is "contains", not "starts with", because operator-created
objects legitimately carry leading words of their own: mongodb excludes
`internal-mongodb-{{ .name }}-users`, which is correct and stays green.
Definitions with an empty prefix are skipped, since `hack/update-crd.sh`
generates the definitions for `packages/extra` charts with `PREFIX=""`.

A selector that is correct and still fails the check can opt out on its
own with a trailing `# cozy-lint:ignore rd-release-prefix -- <reason>`
comment. A directive without a reason does not count.

Verified by mutation: the guard fails on the three harbor selectors
before the fix and passes after. Across the rest of the tree `harbor`
was the only violation.

### What this PR does not do

It does not settle whether the prefix belongs in the template context
instead. That would remove the duplication, but it requires migrating
the definitions that spell the prefix out today, or they would double
it. The guard is the cheaper half and closes the class either way.


### Screenshots

<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->

### Downstream repositories

<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.

Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.

IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:

1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

Trigger map walked against the diff, which touches
`packages/system/harbor-rd/cozyrds/harbor.yaml` and adds
`hack/check-rd-release-prefix.bats`. The two nearest entries were
checked and neither fires:

- **terraform-provider-cozystack** lists "change `release.prefix`, or
rename an output Secret or Service". Neither happens here:
`release.prefix` is untouched and no object is renamed. The provider
concatenates the prefix itself to derive names, so it was already
computing the correct ones — it is this definition's selectors that were
wrong.
- **ccp** lists "rename `packages/system/<name>-rd/cozyrds/<name>.yaml`"
and "move or rename anything under `hack/`". The file is edited, not
renamed, and the new bats file is an addition; no make target changes
behaviour.

**website** would fire on a change to `ApplicationDefinition` semantics.
Semantics are unchanged — one definition's data is corrected — and
`content/en/docs/next/cozystack-api/application-definitions.md`
documents neither `resourceNames` nor the prefix rule, so there is
nothing there to bring in step.


### Release note

<!--  Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->

```release-note
fix(harbor-rd): the ApplicationDefinition selectors for Harbor's Secret, Service and Ingress now carry the release prefix, so they match the objects the chart creates. No user-visible change — the Secret already reached tenants through a label-based workaround, and nothing consumes the Service and Ingress labels.
```



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Bug Fixes**
- Updated Harbor resource naming patterns to consistently include the
`harbor-` prefix for credentials, services, and ingress resources.
- Improved alignment between Harbor resource names and their configured
release prefixes.

- **Tests**
- Added validation to detect missing release prefixes in secret,
service, and ingress resource selectors across system application
definitions.
- Validation reports all affected selectors and supports documented
exceptions where necessary.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/bug Categorizes issue or PR as related to a bug lgtm This PR has been approved by a maintainer size/XS This PR changes 0-9 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants