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

[cosigned] remove regex from the image pattern fields #1873

Merged
merged 1 commit into from
May 14, 2022

Conversation

hectorj2f
Copy link
Contributor

@hectorj2f hectorj2f commented May 13, 2022

Signed-off-by: hectorj2f [email protected]

Summary

As discussed in #1834 we decided to remove regex from the image pattern fields.

Ticket Link

Fixes #1834

Release Note

cosigned: remove regex field from image pattern options.

@hectorj2f hectorj2f added the enhancement New feature or request label May 13, 2022
@hectorj2f hectorj2f self-assigned this May 13, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 13, 2022

Codecov Report

Merging #1873 (872a182) into main (20c8c88) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1873      +/-   ##
==========================================
- Coverage   33.37%   33.31%   -0.06%     
==========================================
  Files         146      146              
  Lines        9378     9370       -8     
==========================================
- Hits         3130     3122       -8     
- Misses       5875     5876       +1     
+ Partials      373      372       -1     
Impacted Files Coverage Δ
pkg/apis/config/image_policies.go 66.66% <ø> (+1.96%) ⬆️
...apis/cosigned/v1alpha1/clusterimagepolicy_types.go 0.00% <ø> (ø)
...cosigned/v1alpha1/clusterimagepolicy_validation.go 93.83% <100.00%> (-0.25%) ⬇️
pkg/cosign/kubernetes/webhook/validator.go 77.89% <100.00%> (+0.19%) ⬆️
pkg/cosign/tuf/client.go 61.68% <0.00%> (-0.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20c8c88...872a182. Read the comment docs.

@hectorj2f hectorj2f requested a review from vaikas May 13, 2022 13:47
@vaikas
Copy link
Contributor

vaikas commented May 13, 2022

Maybe we should create the v1beta1 API first before removing it in there instead?

@hectorj2f
Copy link
Contributor Author

@vaikas i thought the idea was to make these changes before v1beta1. That is why we can do it on alpha versions.

@vaikas
Copy link
Contributor

vaikas commented May 13, 2022

I'm not sure who's all using this, but since it's a breaking change seemed like a safer thing to roll these into v1beta1. At least that's what I've been accustomed to, but if we feel it's ok to introduce breaking changes I'm ok with it. I'll start working on the v1beta1 anyways now, so hopefully get something out by the end of the day that we can iterate on.

@hectorj2f
Copy link
Contributor Author

hectorj2f commented May 14, 2022

@vaikas i was following the indications from kubernetes api versioning where you can make these changes in alpha versions.

Alpha:

The version names contain alpha (for example, v1alpha1).
The software may contain bugs. Enabling a feature may expose bugs. A feature may be disabled by default.
The support for a feature may be dropped at any time without notice.
The API may change in incompatible ways in a later software release without notice.
The software is recommended for use only in short-lived testing clusters, due to increased risk of bugs and lack of long-term support.

You can find that in https://kubernetes.io/docs/reference/using-api/.

@vaikas
Copy link
Contributor

vaikas commented May 14, 2022

@hectorj2f yeah, I know that's what k8s did, we tried to be bit more mindful of breaking changes with Knative, but I'm totes fine with this.

Copy link
Contributor

@vaikas vaikas left a comment

Choose a reason for hiding this comment

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

Would you mind updating the sigstore/sigstore-website with the removal for the docs side of the house?

@hectorj2f
Copy link
Contributor Author

Would you mind updating the sigstore/sigstore-website with the removal for the docs side of the house?

Sure, I'll create the PR.

@hectorj2f hectorj2f merged commit 5d87c9d into sigstore:main May 14, 2022
@hectorj2f hectorj2f deleted the removing_image_pattern_regex branch May 14, 2022 17:12
@github-actions github-actions bot added this to the v1.9.0 milestone May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cosigned] image pattern selector: keep glob and remove regex
3 participants