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

feat: add metadata.annotations to v1alpha1 schema #3044

Closed

Conversation

marshall007
Copy link

Description

This adds metadata.annotations from v1beta1 to the existing v1alpha1 schema. It also adds them to the OCI config descriptor and image manifests.

Related Issue

Relates to #2976.

Checklist before merging

@marshall007 marshall007 requested review from a team as code owners September 27, 2024 19:14
@marshall007 marshall007 changed the title add metadata.annotations to v1alpha1 schema feat: add metadata.annotations to v1alpha1 schema Sep 27, 2024
Copy link

netlify bot commented Sep 27, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 7e847b3
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/66f7046c7d1d1200082706d3

Signed-off-by: Marshall Cottrell <marshall@defenseunicorns.com>
Signed-off-by: Marshall Cottrell <marshall@defenseunicorns.com>
Comment on lines +114 to +116
// annotations explicitly defined in `metadata.annotations` take precedence over legacy fields
maps.Copy(annotations, metadata.Annotations)

Copy link
Author

Choose a reason for hiding this comment

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

I could not find any existing tests for populating annotations on the OCI manifest and config. Happy to add some if there is an obvious place to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah there aren't currently any tests that verify that annotations end up on the manifest or manifest config

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pkg/zoci/push.go 0.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/api/v1alpha1/package.go 75.00% <ø> (ø)
src/api/v1beta1/package.go 75.00% <ø> (ø)
src/pkg/zoci/push.go 0.00% <0.00%> (ø)

Copy link
Member

@phillebaba phillebaba left a comment

Choose a reason for hiding this comment

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

We need to discuss if this should be added to a new API Version or not before merging this.

@schristoff
Copy link
Contributor

Thank you for this contribution, but due to the work we are planning on v1.0 this may conflict with our own implementations.

@schristoff schristoff closed this Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants