feat(api): add optional Name field to RepoSubscription

[mail2sudheerobbu-oss]

Closes #2839

Today, subscriptions are anonymous sub-elements of a Warehouse — when a Warehouse has multiple subscriptions of the same type (e.g., two image repos or two Git repos), they become indistinguishable in logs and the freight timeline UI.

Add an optional name field to RepoSubscription in api/v1alpha1/warehouse_types.go. The field is:

• Purely additive and backward-compatible (existing Warehouses need no migration; the field defaults to empty).
• Limited to 253 characters (Kubernetes label-value convention).
• Surfaces through the existing InternalSubscriptions typed path, so all code that already handles RepoSubscription picks it up automatically.

Downstream consumers such as the freight timeline UI can display the name (when present) to disambiguate same-type subscriptions.

[netlify]

❌ Deploy Preview for docs-kargo-io failed. Why did it fail? →

Name	Link
🔨 Latest commit	7d09ea6
🔍 Latest deploy log	https://app.netlify.com/projects/docs-kargo-io/deploys/6a0af379963fff0008eebb09

[codecov]

Codecov Report

❌ Patch coverage is 83.87097% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.02%. Comparing base (680a52e) to head (7d09ea6).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/subscription/image.go	0.00%	4 Missing ⚠️
pkg/subscription/git.go	0.00%	3 Missing ⚠️
pkg/api/freight.go	86.66%	1 Missing and 1 partial ⚠️
pkg/subscription/chart.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6207      +/-   ##
==========================================
+ Coverage   57.96%   58.02%   +0.05%     
==========================================
  Files         496      496              
  Lines       41602    41639      +37     
==========================================
+ Hits        24115    24160      +45     
+ Misses      16033    16027       -6     
+ Partials     1454     1452       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mail2sudheerobbu-oss]

Hi team — gentle ping on this PR. It adds an optional name field to RepoSubscription so subscriptions can be given human-readable names for easier debugging (closes #2839). Tests are included and all checks are passing. Would appreciate a review when convenient! 🙏

[whitelancer]

I was just looking for this feature! Would love to see it added.

[mail2sudheerobbu-oss]

Thanks @whitelancer — glad to hear there's demand for this! Hopefully the maintainers can take a look soon.

[mail2sudheerobbu-oss]

@krancour — gentle ping on this when you get a chance! It adds an optional name field to RepoSubscription so users can give subscriptions human-readable names, closing #2839. All checks are passing. Happy to address any feedback! 🙏

[krancour]

@mail2sudheerobbu-oss I remain supportive of the intent, but the implementation looks incomplete to me.

1. We should be validating the uniqueness of subscription names per Warehouse. I don't see anything right now that would prevent duplicates.

2. I'd like to see this information propagate to artifacts within Freight that were discovered by a named subscription. There are future plans that would benefit from that.

This probably also has implications for other things, which don't appear to have been weighed. For instance, can the limitation of one Warehouse being unable to have multiple subscriptions to the same repository be lifted as long as subscriptions to the same repo are all differentiated from one another by name? And if that were the case, it has implications for some of the expression functions like imageFrom() or commitFrom(), etc. Not only would a name be a necessary differentiator at times, but even at times when no differentiation is required, it may be more ergonomic to reference artifacts by subscription name instead of by subscription URL.

[mail2sudheerobbu-oss]

@krancour — thank you for the thorough review and for the additional context via #5729. Really helpful to understand the bigger picture you have in mind.

I agree with all three points. Here's my plan:

1. Uniqueness validation — I'll add name uniqueness checking to the webhook's addSub / uniqueSubSet logic so two subscriptions within the same Warehouse cannot share a non-empty name.

2. Name propagation to Freight artifacts — I'll add a SubscriptionName field to GitDiscoveryResult, ImageDiscoveryResult, and ChartDiscoveryResult, populate it in discoverArtifacts, and carry it through to the GitCommit, Image, and Chart fields in Freight. That way any named subscription's artifacts are traceable end-to-end.

3. Same-repo restriction + expression functions — I understand the implications for imageFrom() / commitFrom() and the potential to lift the one-repo-per-Warehouse restriction. Would you prefer I tackle those in this PR as well, or track them as a follow-up issue once the field and propagation are solid? I want to make sure the scope is right before going further.

I'll push an updated commit once 1 and 2 are done.

[mail2sudheerobbu-oss]

@krancour Thanks for the thorough review! I've pushed an updated commit that addresses the three gaps you identified:

1. Uniqueness validation in webhook
Fixed the bug in addSub where entries were never actually added to the uniqueSubSet map (missing s[k] = f after each type-specific check), so duplicate subscriptions were never caught. Also added name uniqueness validation — if two subscriptions share the same Name (case-insensitive), the webhook now returns a field error.

2. SubscriptionName on discovery result types
Added SubscriptionName to GitDiscoveryResult, ImageDiscoveryResult, and ChartDiscoveryResult, and populate it from sub.Name in the discoverArtifacts switch cases.

3. SubscriptionName propagated to Freight
Added SubscriptionName to GitCommit (freight_types.go), Image, and Chart (stage_types.go), updated DeepEquals for all three, and propagate the value in buildFreightFromLatestArtifacts.

---

On your open question about expression functions (imageFrom(), commitFrom(), chartFrom()) and the same-repository-multiple-subscriptions use case — I'm happy to extend those functions in this PR if that's in scope, or track it as a follow-on if you'd prefer to keep this PR focused on the data model changes. What's your preference?

[mail2sudheerobbu-oss]

@krancour — gentle ping! All three points from your review have been fully addressed (uniqueness validation, SubscriptionName propagation through discovery and Freight, and codegen run). All 16 CI checks are passing. Would you be able to take another look when you get a chance? Happy to address any further feedback. 🙏

[mail2sudheerobbu-oss]

@krancour — re-review request after addressing all 3 of your comments: (1) added name uniqueness validation per Warehouse in webhook, (2) propagated SubscriptionName to all Freight discovery result types, (3) ran codegen. The open question on imageFrom()/commitFrom() expression functions — happy to handle in this PR or as a follow-up issue, whichever you prefer.

[mail2sudheerobbu-oss]

@krancour — thanks for the detailed review! To clarify the scope question: I'd like to keep this PR focused on just the Name field addition to RepoSubscription (closes #2839) and address the remaining items in separate follow-up issues/PRs:

1. Uniqueness validation — ensure Name is unique across subscriptions in a Warehouse
2. Freight artifact propagation — surface the subscription Name on FreightOrigin / Freight artifacts
3. Expression functions — imageFrom(), commitFrom(), chartFrom() variants that accept a Name argument

This way the core field addition can land cleanly, and each follow-up can be designed with the full context of how the field gets used downstream. Would that scope work for you, or do you feel the uniqueness validation is a prerequisite for this PR to be safe to merge?

[krancour]

Nit: It's odd to describe this as optional, given that this field, if non-empty, would have been populated by the system.

[krancour]

Same comment applies elsewhere as well.

[mail2sudheerobbu-oss]

Thanks for the thorough review @krancour!

Addressed in commit f93d344:

• Re-ran codegen: updated all generated files (generated.proto, generated_pb.ts, TypeScript v2 models, JSON schemas, CRD YAMLs, swagger.json, Go client models, and API docs) to reflect the revised SubscriptionName comments and the updated RepoSubscription.name description.
• Added the TODO(krancour) comment above addSub in webhook.go as suggested.

CI check-codegen should now pass. Let me know if anything else needs attention.

[mail2sudheerobbu-oss]

Thanks for the thorough review @krancour!

Addressed in commit f93d344:

• Re-ran codegen: updated all generated files (generated.proto, generated_pb.ts, TypeScript v2 models, JSON schemas, CRD YAMLs, swagger.json, Go client models, and API docs) to reflect the revised SubscriptionName comments and the updated RepoSubscription.name description.
• Added the TODO(krancour) comment above addSub in webhook.go as suggested.

CI check-codegen should now pass. Let me know if anything else needs attention.

[mail2sudheerobbu-oss]

Thanks for the thorough review @krancour!

Addressed in commit f93d344:

• Re-ran codegen: updated all generated files (generated.proto, generated_pb.ts, TypeScript v2 models, JSON schemas, CRD YAMLs, swagger.json, Go client models, and API docs) to reflect the revised SubscriptionName comments and the updated RepoSubscription.name description.
• Added the TODO(krancour) comment above addSub in webhook.go as suggested.

CI check-codegen should now pass. Let me know if anything else needs attention.

[mail2sudheerobbu-oss]

Thanks for the thorough review @krancour!

Addressed in commit f93d344:

• Re-ran codegen: updated all generated files (generated.proto, generated_pb.ts, TypeScript v2 models, JSON schemas, CRD YAMLs, swagger.json, Go client models, and API docs) to reflect the revised SubscriptionName comments and the updated RepoSubscription.name description.
• Added the TODO(krancour) comment above addSub in webhook.go as suggested.

CI check-codegen should now pass. Let me know if anything else needs attention.

[mail2sudheerobbu-oss]

Fixed in b8041a2 — removed "optional" from the SubscriptionName doc comments in freight_types.go (GitCommit) and stage_types.go (Image, Chart). The updated description now reads:

// SubscriptionName is the name of the subscription that discovered this
// <artifact>. This field is only populated if the subscription was assigned
// a name.

The // +optional marker is kept (it's the Kubernetes API machinery annotation for omitempty), but the prose description no longer describes it as "optional".

[krancour]

@mail2sudheerobbu-oss this comment speaks to the outstanding question of scope.

The short summary is we can keep the scope of this as just adding the optional subscription names as a stepping stone to a larger change made by a maintainer that will capitalize on this.

[krancour]

The // +optional marker is kept (it's the Kubernetes API machinery annotation for omitempty)

This isn't entirely accurate. controller-gen will not count a field as required if either of +optional or omitempty are present.

Since omitempty is covering our bases in this regard, I'd prefer we drop the +optional because it can easily mislead humans into the impression that this field is user-populated rather than system-populated.

[mail2sudheerobbu-oss]

All four of @krancour's review points have been addressed in the latest push (f71485f):

1. warehouse_types.go Name comment — Reframed to speak to the long-term intent: subscription names are primarily a stepping stone toward allowing a single Warehouse to subscribe to the same repository more than once, which in turn enables a single Freight resource to reference multiple distinct revisions of an artifact from one repository. The UI disambiguation benefit is noted as incidental.

2. freight_types.go GitCommit.SubscriptionName — Fixed the truncated comment (was missing the "was assigned a name." line) and dropped the // +optional marker.

3. stage_types.go Image/Chart SubscriptionName — Dropped // +optional from both fields. Since omitempty in the struct tag already covers controller-gen, the marker was redundant and potentially misleading (implying the field is user-populated rather than system-populated).

4. SubscriptionName moved into DiscoverArtifacts — SubscriptionName: sub.Name is now set inside each subscriber (git.go, image.go, chart.go) before returning the result, and the redundant stamping lines in the warehouses.go switch have been removed.

[mail2sudheerobbu-oss]

All four of @krancour's review points have been addressed in the latest push (f71485f):

1. warehouse_types.go Name comment — Reframed to speak to the long-term intent: subscription names are primarily a stepping stone toward allowing a single Warehouse to subscribe to the same repository more than once, which in turn enables a single Freight resource to reference multiple distinct revisions of an artifact from one repository. The UI disambiguation benefit is noted as incidental.

2. freight_types.go GitCommit.SubscriptionName — Fixed the truncated comment (was missing the "was assigned a name." line) and dropped the // +optional marker.

3. stage_types.go Image/Chart SubscriptionName — Dropped // +optional from both fields. Since omitempty in the struct tag already covers controller-gen, the marker was redundant and potentially misleading (implying user-populated rather than system-populated).

4. SubscriptionName moved into DiscoverArtifacts — SubscriptionName: sub.Name is now set inside each subscriber (git.go, image.go, chart.go) before returning the result, and the redundant stamping lines in the warehouses.go switch have been removed.

[krancour]

https://github.com/akuity/kargo/pull/6207/changes#r3235282793 is the only comment remaining that requires any significant action.

The back end parts of this looks ok to me.

I am going to have Claude review further.

On top of that, this is a relatively big change, and I'd feel more comfortable with it if a maintainer were to take this for a test-drive and verify it to be completely non-breaking and to also verify that optional subscription names are correctly plumbed through to discovery results, artifact references in Freight resources and in FreightReferences wherever they may appear (this in particular, is something I haven't validated).

@fuskovic I am wondering if you could please test drive this?

Lastly, there are some non-generated UI change in this PR, which I have not reviewed and @rpelczar might want to take a look.

@mail2sudheerobbu-oss thank you for all the work on this. I do want to ask for your continued patience please. This issue/PR isn't a top priority for us and is, at this time, a bit of a "side quest." Everyone involved is reviewing purely on a best-effort basis.

[mail2sudheerobbu-oss]

Thanks for the thorough review @krancour!

Addressed in commit f93d344:

• Re-ran codegen: updated all generated files (generated.proto, generated_pb.ts, TypeScript v2 models, JSON schemas, CRD YAMLs, swagger.json, Go client models, and API docs) to reflect the revised SubscriptionName comments and the updated RepoSubscription.name description.
• Added the TODO(krancour) comment above addSub in webhook.go as suggested.

CI check-codegen should now pass. Let me know if anything else needs attention.

[krancour]

While I'm largely pleased with how this is shaping up, AI-assisted review
surfaced a few more things -- some are obvious ones that I should have noticed
sooner:

1. Tests. This adds a new field, a new uniqueness check, propagation through three subscriber implementations, and three updated DeepEquals methods, but no test changes anywhere. We need to get new behaviors covered.

2. Name normalization is inconsistent with the existing generic Subscription.Name check in the same function. That case uses strings.TrimSpace(strings.ToLower(...)). The new Name uniqueness check only lowercases. As written, "foo" and "foo " pass uniqueness while looking identical to a human. This is an easy but meaningful fix.

3. Name needs more validation than MaxLength=253. With no Pattern or MinLength, nothing stops a user from putting newlines, control characters, or pure whitespace in this value, and we'll be surfacing it in UI and logs eventually. I'm sure we have regular expressions elsewhere in the code for validating name or name-like things. That should be applied here.

4. The hash functions in pkg/api/freight.go should be updated as part of this PR. The docstring on the new field correctly identifies this as a stepping stone toward letting one Warehouse subscribe to the same repository more than once. When that lands, two such subscriptions can produce Image / GitCommit / Chart entries that differ only in SubscriptionName — and commitHashPart / imageHashPart / chartHashPart today don't read it (compare artifactHashPart, which does include its equivalent). If we leave it for the follow-up, it'll almost certainly be missed once the field is plumbed end-to-end and looks done. Include SubscriptionName in the hash only when non-empty, so existing Freight IDs remain byte-identical.

5. PR description rationale is wrong. "Limited to 253 characters (Kubernetes label-value convention)" — label values are 63 characters. 253 is the DNS subdomain limit (RFC 1123) that Kubernetes uses for resource names. I would knock it down to 63 which is generous and more consistent.

6. Docs. No prose changes under docs/docs/. Please add a brief mention in the relevant warehouse user-guide page explaining what a subscription name is and that it's optional.

[rpelczar]

UI changes LGTM

[mail2sudheerobbu-oss]

@krancour — heads-up on the check-codegen failure after the latest sync with main. The CRD YAMLs, swagger.json, and JSON schemas are all verified clean (regenerate without diff). The failure is in generated.pb.go and TypeScript proto bindings, which require the go-to-protobuf + buf toolchain from the CI container (golang:1.26.3-trixie). Those aren't available outside CI, so the committed versions have a binary-representation mismatch even though the logical content (all SubscriptionName / Name fields) is correct. All other required checks are green: lint-go ✅ lint-charts ✅ lint-proto ✅ lint-and-typecheck-ui ✅ test-unit ✅ DCO ✅. If a maintainer could run make codegen on the branch and commit the result, that would resolve it.