Fix panic in manifest annotate --index
#24776
Conversation
test/e2e/manifest_test.go
Outdated
| Expect(session.OutputToString()).To(ContainSubstring(`"hello": "world,withcomma"`)) | ||
| Expect(session.OutputToString()).To(ContainSubstring(`"left": "right"`)) |
There was a problem hiding this comment.
Doesn't both annotation are set on different levels? I feel like such a test must make sure they are set in the correct places not just that they exists anywhere.
There was a problem hiding this comment.
Yes, they are. Reworked the comparison logic.
nalind
force-pushed
the
manifest-annotate-index
branch
from
03eea16 to
45a4edf
Compare
test/e2e/manifest_test.go
Outdated
| { | ||
| // from imageListInstance | ||
| MediaType: manifest.DockerV2Schema2MediaType, | ||
| Digest: digest.Digest("sha256:1385ce282f3a959d0d6baf45636efe686c1e14c3e7240eb31907436f7bc531fa"), |
There was a problem hiding this comment.
to not duplicate this needs to use imageListARM64InstanceDigest
There was a problem hiding this comment.
Changed to parse it from the name that we use when adding it to the index.
nalind
force-pushed
the
manifest-annotate-index
branch
from
45a4edf to
a28d167
Compare
|
Looks like --index doesn't work with podman-remote at all. At least tests seem failing and I cannot see any use of opts.IndexAnnotations in the remote client bindings. |
nalind
force-pushed
the
manifest-annotate-index
branch
from
a44a559 to
f1204b7
Compare
github-actions
bot
added
the
kind/api-change
When the --index flag is used, `manifest annotate` shouldn't be expecting a second non-flag argument. Signed-off-by: Nalin Dahyabhai <[email protected]>
Add the connective logic so that annotating the manifest as a whole will succeed as intended, and we don't mix up annotations for an entry and annotations which are meant for the manifest as a whole. Make consistent the names which are used when encoding values of certain fields. Signed-off-by: Nalin Dahyabhai <[email protected]>
nalind
force-pushed
the
manifest-annotate-index
branch
2 times, most recently
from
fcbeb1e to
0a48856
Compare
openshift-ci
bot
added
the
approved
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, nalind, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
When the --index flag is used,
manifest annotateshouldn't be expecting a second non-flag argument.Fixes #24750.
Does this PR introduce a user-facing change?