Skip to content

fix: rename ManifestAddArtifactOptions.Annotations to ArtifactAnnotations #28981

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.

Sign up for GitHub

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

Open
stefan8 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: rename ManifestAddArtifactOptions.Annotations to ArtifactAnnotations #28981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions pkg/api/handlers/libpod/manifests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,14 +674,14 @@ func ManifestModify(w http.ResponseWriter, r *http.Request) {
// We waited until after the extraction goroutine finished to ensure
// that we'd pick up its changes to the ArtifactFiles list.
manifestAddArtifactOptions := entities.ManifestAddArtifactOptions{
Type: body.ArtifactType,
LayerType: body.ArtifactLayerType,
ConfigType: body.ArtifactConfigType,
Config: body.ArtifactConfig,
ExcludeTitles: body.ArtifactExcludeTitles,
Annotations: body.ArtifactAnnotations,
Subject: body.ArtifactSubject,
Files: body.ArtifactFiles,
Type: body.ArtifactType,
LayerType: body.ArtifactLayerType,
ConfigType: body.ArtifactConfigType,
Config: body.ArtifactConfig,
ExcludeTitles: body.ArtifactExcludeTitles,
ArtifactAnnotations: body.ArtifactAnnotations,
Subject: body.ArtifactSubject,
Files: body.ArtifactFiles,
}
id, err := imageEngine.ManifestAddArtifact(r.Context(), name, body.ArtifactFiles, manifestAddArtifactOptions)
if err != nil {
Expand Down
17 changes: 9 additions & 8 deletions pkg/domain/entities/manifest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,15 @@ type ManifestAddOptions struct {
type ManifestAddArtifactOptions struct {
ManifestAnnotateOptions
// Note to future maintainers: keep these fields synchronized with ManifestModifyOptions!
Type *string `json:"artifact_type" schema:"artifact_type"`
LayerType string `json:"artifact_layer_type" schema:"artifact_layer_type"`
ConfigType string `json:"artifact_config_type" schema:"artifact_config_type"`
Config string `json:"artifact_config" schema:"artifact_config"`
ExcludeTitles bool `json:"artifact_exclude_titles" schema:"artifact_exclude_titles"`
Annotations map[string]string `json:"artifact_annotations" schema:"artifact_annotations"`
Subject string `json:"artifact_subject" schema:"artifact_subject"`
Files []string `json:"artifact_files" schema:"-"`
Type *string `json:"artifact_type" schema:"artifact_type"`
LayerType string `json:"artifact_layer_type" schema:"artifact_layer_type"`
ConfigType string `json:"artifact_config_type" schema:"artifact_config_type"`
Config string `json:"artifact_config" schema:"artifact_config"`
ExcludeTitles bool `json:"artifact_exclude_titles" schema:"artifact_exclude_titles"`
// ArtifactAnnotations has a distinct Go name (the tag is unchanged) to avoid a swagger x-go-name collision with the embedded ManifestAnnotateOptions.Annotations.
ArtifactAnnotations map[string]string `json:"artifact_annotations" schema:"artifact_annotations"`
Subject string `json:"artifact_subject" schema:"artifact_subject"`
Files []string `json:"artifact_files" schema:"-"`
}

// ManifestAnnotateOptions provides model for annotating manifest list
Expand Down
2 changes: 1 addition & 1 deletion pkg/domain/infra/abi/manifest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func (ir *ImageEngine) ManifestAddArtifact(ctx context.Context, name string, fil
Config: opts.Config,
LayerType: opts.LayerType,
ExcludeTitles: opts.ExcludeTitles,
Annotations: opts.Annotations,
Annotations: opts.ArtifactAnnotations,

@Honny1 Honny1 Jun 22, 2026

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.

Is this correct? It will set ArtifactAnnotation into the embedded field Annotations.

@stefan8 stefan8 Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, correct. The Annotations: field on this line belongs to libimage.ManifestListAddArtifactOptions (the addArtifactOptions literal at L358), not to the embedded entities.ManifestAnnotateOptions. So it sets the artifact's annotations — the same destination they had before this PR.

The value is unchanged: before the rename, ManifestAddArtifactOptions had a direct Annotations field that shadowed the embedded ManifestAnnotateOptions.Annotations (both map[string]string), so opts.Annotations here already resolved to the artifact field (json artifact_annotations). The rename keeps that json/schema tag, so opts.ArtifactAnnotations is the identical field — only the Go symbol is now unambiguous.

The manifest-list (index) annotations travel a separate path and are untouched: they're read via the fully-qualified opts.ManifestAnnotateOptions.Annotations at L382 into ManifestListAnnotateOptions. That qualified access already existed precisely because the direct field used to shadow the embedded one, so the rename can't orphan or misroute it.

Net: artifact annotations → artifact options (this line); manifest annotations → annotate options (L382). The rename only removes the duplicate x-go-name: Annotations in the generated swagger model — no behavior change.

Subject: opts.Subject,
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/domain/infra/tunnel/manifest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (ir *ImageEngine) ManifestAddArtifact(_ context.Context, name string, files
options.WithType(opts.Type).WithConfigType(opts.ConfigType).WithLayerType(opts.LayerType)
options.WithConfig(opts.Config)
options.WithExcludeTitles(opts.ExcludeTitles).WithSubject(opts.Subject)
options.WithAnnotations(opts.Annotations)
options.WithAnnotations(opts.ArtifactAnnotations)

@Honny1 Honny1 Jun 22, 2026

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.

Is this correct?

@stefan8 stefan8 Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same reasoning — WithAnnotations(opts.ArtifactAnnotations) sets the artifact-add binding's annotations. It previously read opts.Annotations, which via the shadowing above was the same artifact field, so the value is identical (see my reply on abi/manifest.go:364).

options.WithFiles(files)
id, err := manifests.AddArtifact(ir.ClientCtx, name, options)
if err != nil {
Expand Down