Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document the possible values of Warehouse imageSelectionStrategy #2880

Closed
1 of 3 tasks
Chewie opened this issue Oct 30, 2024 · 3 comments · Fixed by #2969 · May be fixed by #2963
Closed
1 of 3 tasks

Document the possible values of Warehouse imageSelectionStrategy #2880

Chewie opened this issue Oct 30, 2024 · 3 comments · Fixed by #2969 · May be fixed by #2963

Comments

@Chewie
Copy link

Chewie commented Oct 30, 2024

Checklist

  • I've searched the issue queue to verify this is not a duplicate feature request.
  • I've pasted the output of kargo version, if applicable.
  • I've pasted logs, if applicable.

Proposed Feature

Add an entry in the docs for the possible values of imageSelectionStrategy, namely Digest, Lexical and NewestBuild.

Motivation

Playing around with Kargo for evaluation, I wanted to try a toy continuous delivery scenario without semver:

  • A push on main on the app repo triggers tests and a new image build
  • That new image is immediately deployed to a staging cluster
  • Some sort of E2E tests and/or human validation is made and triggers the next stage
  • The new version is now deployed in production

However, I found it difficult to find the information to represent this scenario. The key concepts kind of implies that a semver approach is recommended, and while the CRD Reference shows an imageSelectionStrategy field, it doesn't show what the possible values are.

I was only able to discover the possible values Digest, Lexical and NewestBuild by looking at the old 0.4.0 release notes.

However, I'm not sure if this is an oversight or an information hidden on purpose. If it is the latter, what would be the recommended approach to implement this scenario? On a more general note, I'd be interested to know if the project has strong opinions regarding promotion strategies, in particular those that want to move away from tagging releases.

Cheers!

@krancour
Copy link
Member

Thanks for opening this issue. You're the second to notice this in as many days.

It wasn't deliberate.

It's a combination of hand-written docs still evolving and us being taken by surprise that the enumerated accepted values for this field were not included in the generated documentation.

For reference, we had incorrectly expected this:

// +kubebuilder:validation:Enum={Digest,Lexical,NewestBuild,SemVer}
type ImageSelectionStrategy string

To surface here:

https://doc.crds.dev/github.com/akuity/kargo/kargo.akuity.io/Warehouse/[email protected]#spec-subscriptions-image-imageSelectionStrategy

@krancour
Copy link
Member

The same issue exists for commitSelectionStrategy -- the generated docs do not show the possible values.

Ref: #2955 (comment)

Both should be fixed at the same time.

Increasing priority and assigning.

@krancour krancour removed good first issue Good issue for a new contributor to handle help-wanted Community help on this would be appreciated labels Nov 18, 2024
@krancour krancour added this to the v1.1.0 milestone Nov 18, 2024
@fykaa
Copy link
Contributor

fykaa commented Nov 20, 2024

From my observation, the enumerated accepted values for fields have consistently not been reflected in the generated documentation. This issue persists across all similar fields. For example, in warehouses_types.go:

  • freightCreationPolicy: Automatic, Manual
  • commitSelectionStrategy: Lexical, NewestFromBranch, NewestTag, SemVer
  • imageSelectionStrategy: Digest, Lexical, NewestBuild, SemVer
  • status: True, False, Unknown

These values are present in the JSON but do not appear in the generated documentation

This needs a fix with all other *_types.go as well, i.e. stages, projects, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment