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

buildah manifest: add artifact-related options #5301

Merged
merged 11 commits into from
Feb 7, 2024

Conversation

nalind
Copy link
Member

@nalind nalind commented Jan 30, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add functionality for creating artifact manifests and adding them to image indexes. buildah manifest add gets a --artifact option for telling it to create artifact manifests, and --artifact-type, --artifact-config-type, --artifact-layer-type, --artifact-exclude-titles, and --subject options to fine-tune the contents of the artifact manifests it creates.

Add a --index flag to buildah manifest annotate so that it can be told to set annotations on the index itself instead of on one of the entries in the image index.

Add a --subject flag to buildah manifest annotate for setting the subject field of an image index.

Add a --annotation flag to buildah manifest create to allow for adding annotations to the new image index.

How to verify it

New integration tests!

Which issue(s) this PR fixes:

Fixes #5051

Special notes for your reviewer:

Does this PR introduce a user-facing change?

`buildah manifest add` now accepts a `--artifact` option, which can create artifact manifests for arbitrary files and add those artifact manifests to an image index.
`buildah manifest annotate` now accepts a `--index` option, indicating that `--annotation` values should be added to the image index rather than an item listed in an image index.

@openshift-ci openshift-ci bot added kind/feature Categorizes issue or PR as related to a new feature. approved labels Jan 30, 2024
@nalind
Copy link
Member Author

nalind commented Jan 30, 2024

/hold until after containers/common#1833 is merged, so that it doesn't depend on a branch in my personal fork.

@nalind nalind force-pushed the fun-with-artifacts branch 2 times, most recently from adbf537 to 70a6313 Compare January 30, 2024 23:12
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@rhatdan
Copy link
Member

rhatdan commented Feb 7, 2024

Can you change the common PR above to not be Do not merge? I think common is up 2 date.

@nalind
Copy link
Member Author

nalind commented Feb 7, 2024

Removing the update of github.com/containers/common from the first commit in the series.

@nalind
Copy link
Member Author

nalind commented Feb 7, 2024

Oh, you meant "take the DO NOT MERGE off the description", since the previously-vendored version of common is from November.

Update github.com/openshift/imagebuilder to the v1.2.6 release
Update github.com/containers/common to the current tip of the main branch

Signed-off-by: Nalin Dahyabhai <[email protected]>
github.com/docker/docker/client.NewVersionError() takes a context.Context now.

Signed-off-by: Nalin Dahyabhai <[email protected]>
... in an attempt to try to get UID 0 in a user namespace to stop trying
to read files from root's home directory, where the permissions error is
treated as a hard failure.

Signed-off-by: Nalin Dahyabhai <[email protected]>
... which will have netavark installed in it, so that tests won't just
fail because it isn't there.

Signed-off-by: Nalin Dahyabhai <[email protected]>
In tests/validate/pr-should-include-tests and
tests/validate/whitespace.sh, use grep -E instead of egrep, because
egrep keeps telling us to switch.

Signed-off-by: Nalin Dahyabhai <[email protected]>
github.com/cilium/ebpf v0.12.3 (the latest tag as of this moment) won't
build on linux/loong64, but the tip of its main branch does.  When
v0.12.4 is released, and we're using that or a later version, we can
turn it back on.

Signed-off-by: Nalin Dahyabhai <[email protected]>
... instead of github.com/containers/common/pkg/util.StringInSlice,
per linters.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Locally-defined structs don't need to be aliases for themselves.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Lock an image index's image before attempting to modify or push it.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Add functionality for creating artifact manifests and adding them to
image indexes.  `buildah manifest add` gets a `--artifact` option for
telling it to create artifact manifests, and `--artifact-type`,
`--artifact-config`, `--artifact-config-type`, `--artifact-layer-type`,
`--artifact-exclude-titles`, and `--subject` options to fine-tune the
contents of the artifact manifests it creates.

Add a `--index` flag to `buildah manifest annotate` so that it can be
told to set annotations on the index itself instead of on one of the
entries in the image index.

Add a `--subject` flag to `buildah manifest annotate` for setting the
`subject` field of an image index.

Add a `--annotation` flag to `buildah manifest create` to allow for
adding annotations to the new image index.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@@ -511,6 +516,9 @@ func runUsingChroot(spec *specs.Spec, bundlePath string, ctty *os.File, stdin io
cmd.Stdin, cmd.Stdout, cmd.Stderr = stdin, stdout, stderr
cmd.Dir = "/"
cmd.Env = []string{fmt.Sprintf("LOGLEVEL=%d", logrus.GetLevel())}
if _, ok := os.LookupEnv(containersConfEnv); ok {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Why not use the val passed back rather then calling os.Getenv() again.if

val, ok := os.LookupEnv(containersConfEnv); ok {
	cmd.Env = append(cmd.Env, containersConfEnv+"="+val))
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that would have been better.

Copy link
Member

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

openshift-ci bot commented Feb 7, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@baude
Copy link
Member

baude commented Feb 7, 2024

i verified it worked with multiple artifacts and quay ... great work @nalind

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Feb 7, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 817237a into containers:main Feb 7, 2024
35 checks passed
@nalind nalind deleted the fun-with-artifacts branch February 7, 2024 20:20
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved kind/feature Categorizes issue or PR as related to a new feature. lgtm locked - please file new issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image index media type and annotation
3 participants