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

Start of the necessary pieces to get #1418 and #1419 implemented #1562

Merged
merged 6 commits into from
Mar 9, 2022

Conversation

vaikas
Copy link
Contributor

@vaikas vaikas commented Mar 8, 2022

This is the start of the necessary pieces to get #1418 and #1419 implemented

ClusterImagePolicy reconciler will now create a configmap (no secret support yet)
and update it on changes (not on deletions yet). Also put up most necessary
testing pieces so that we can start unit testing the reconciler and make sure
it updates the resulting configmap.

There's also a ConfigStore that we can then inject into the admission webhook
that I have wired in there (nop for now, but demonstrating how it could work).
Idea being that you could then for a given image ask for all the authorities that
need to be validated. You can see what that config looks like in the
/pkg/apis/config/testdata/config-image-policies.yaml and the accompanying tests
in /pkg/apis/config/image_policies_test
I made sure that it works with both yaml/json.

While playing with this there's some questions that came to mind, so I'll take
those to the document.

Hope is that we get enough pieces in place so that we can agree on the major
moving pieces and how they fit together and enough testing in place that
we can start sharding up the work more efficiently and in more focused areas.

Signed-off-by: Ville Aikas vaikas@chainguard.dev

Summary

Ticket Link

Fixes

Release Note


@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2022

Codecov Report

Merging #1562 (ab90647) into main (e1f7c78) will increase coverage by 0.90%.
The diff coverage is 67.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1562      +/-   ##
==========================================
+ Coverage   26.49%   27.39%   +0.90%     
==========================================
  Files         126      130       +4     
  Lines        7214     7376     +162     
==========================================
+ Hits         1911     2021     +110     
- Misses       5093     5132      +39     
- Partials      210      223      +13     
Impacted Files Coverage Δ
cmd/cosign/webhook/main.go 0.00% <0.00%> (ø)
pkg/cosign/kubernetes/webhook/validation.go 74.02% <27.27%> (-7.80%) ⬇️
...econciler/clusterimagepolicy/clusterimagepolicy.go 44.73% <43.24%> (ø)
pkg/apis/config/image_policies.go 71.42% <71.42%> (ø)
pkg/reconciler/clusterimagepolicy/controller.go 87.50% <83.33%> (ø)
pkg/apis/config/store.go 100.00% <100.00%> (ø)
pkg/cosign/tuf/client.go 62.34% <0.00%> (-0.95%) ⬇️

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 e1f7c78...ab90647. Read the comment docs.

cmd/cosign/webhook/main.go Outdated Show resolved Hide resolved
@vaikas
Copy link
Contributor Author

vaikas commented Mar 8, 2022

I don't understand why codegen is failing, I rebased to head, ran update-codegen and it shows no changes.

@vaikas
Copy link
Contributor Author

vaikas commented Mar 8, 2022

Need to grant permissions to look at the configmap and secrets

2022-03-08T14:26:56.051682941Z stderr F E0308 14:26:56.051593       1 reflector.go:138] k8s.io/client-go@v0.23.3/tools/cache/reflector.go:167: Failed to watch *v1.ConfigMap: failed to list *v1.ConfigMap: configmaps is forbidden: User "system:serviceaccount:cosign-system:webhook" cannot list resource "configmaps" in API group "" at the cluster scope
2022-03-08T14:26:56.051892343Z stderr F E0308 14:26:56.051811       1 reflector.go:138] k8s.io/client-go@v0.23.3/tools/cache/reflector.go:167: Failed to watch *v1.Secret: failed to list *v1.Secret: secrets is forbidden: User "system:serviceaccount:cosign-system:webhook" cannot list resource "secrets" in API group "" at the cluster scope

…gstore#1419 implemented

ClusterImagePolicy reconciler will now create a configmap (no secret support yet)
and update it on changes (not on deletions yet). Also put up most necessary
testing pieces so that we can start unit testing the reconciler and make sure
it updates the resulting configmap.

There's also a ConfigStore that we can then inject into the admission webhook
that I have wired in there (nop for now, but demonstrating how it could work).
Idea being that you could then for a given image ask for all the authorities that
need to be validated. You can see what that config looks like in the
/pkg/apis/config/testdata/image-policies.yaml and the accompanying tests
in /pkg/apis/config/image_policies_test
I made sure that it works with both yaml/json.

While playing with this there's some questions that came to mind, so I'll take
those to the document.

Hope is that we get enough pieces in place so that we can agree on the major
moving pieces and how they fit together and enough testing in place that
we can start sharding up the work more efficiently and in more focused areas.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Rename to be consistent with the other cm. image-policies => config-image-policies

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
…sions.

Ran manual tests validating that things are working, when I remove
things from the configmap, things are patched back in (after the global
resync is triggered).

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
@vaikas
Copy link
Contributor Author

vaikas commented Mar 9, 2022

I'm so very baffled as to what's going on with hack/update-codegen. Here's what I did (again) and it's still complaining.

vaikas@villes-mbp cosign % git status
On branch cip-reconciler
nothing to commit, working tree clean
vaikas@villes-mbp cosign % git fetch upstream main
From github.com:sigstore/cosign
 * branch            main       -> FETCH_HEAD
vaikas@villes-mbp cosign % git rebase -ff upstream/main
Current branch cip-reconciler is up to date, rebase forced.
Successfully rebased and updated refs/heads/cip-reconciler.
vaikas@villes-mbp cosign % git status
On branch cip-reconciler
nothing to commit, working tree clean
vaikas@villes-mbp cosign % ./hack/update-codegen.sh
~/projects/go/src/github.com/sigstore/cosign ~/projects/go/src/github.com/sigstore/cosign
=== Vendoring scripts
=== Update Codegen for github.com/sigstore/cosign
--- Kubernetes Codegen
Generating deepcopy funcs
Generating clientset for cosigned:v1alpha1 at github.com/sigstore/cosign/pkg/client/clientset
Generating listers for cosigned:v1alpha1 at github.com/sigstore/cosign/pkg/client/listers
Generating informers for cosigned:v1alpha1 at github.com/sigstore/cosign/pkg/client/informers
--- Knative Codegen
Generating injection for cosigned:v1alpha1 at github.com/sigstore/cosign/pkg/client/injection
--- Update CRD Schema
--- Update deps post-codegen
~/projects/go/src/github.com/sigstore/cosign ~/projects/go/src/github.com/sigstore/cosign
=== Vendoring scripts
=== Update Deps for Golang
--- Go mod tidy and vendor
--- Removing unwanted vendor files
--- Updating licenses
--- Removing broken symlinks
=== Removing vendor/
vaikas@villes-mbp cosign % git status
On branch cip-reconciler
nothing to commit, working tree clean
vaikas@villes-mbp cosign % git push origin cip-reconciler --force
Enumerating objects: 121, done.
Counting objects: 100% (121/121), done.
Delta compression using up to 8 threads
Compressing objects: 100% (89/89), done.
Writing objects: 100% (100/100), 25.97 KiB | 3.71 MiB/s, done.
Total 100 (delta 53), reused 1 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (53/53), completed with 14 local objects.
To github.com:vaikas/cosign.git
 + 8df8368...d7c5e58 cip-reconciler -> cip-reconciler (forced update)

@@ -85,6 +87,9 @@ var types = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
}

func NewValidatingAdmissionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
// Decorate contexts with the current state of the config.
Copy link
Contributor

Choose a reason for hiding this comment

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

@vaikas Shouldn't you do the same for the NewMutatingAdmisisonController ?

Copy link
Contributor Author

@vaikas vaikas Mar 9, 2022

Choose a reason for hiding this comment

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

lol, I took it out as per previous PR feedback :) Is there anything in the policy that gets used by mutating?
#1562 (comment)

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
@mattmoor mattmoor merged commit 70d9b9b into sigstore:main Mar 9, 2022
@github-actions github-actions bot added this to the v1.7.0 milestone Mar 9, 2022
@vaikas vaikas deleted the cip-reconciler branch March 9, 2022 15:45
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
…implemented (sigstore#1562)

* This is the start of the necessary pieces to get sigstore#1418 and sigstore#1419 implemented

ClusterImagePolicy reconciler will now create a configmap (no secret support yet)
and update it on changes (not on deletions yet). Also put up most necessary
testing pieces so that we can start unit testing the reconciler and make sure
it updates the resulting configmap.

There's also a ConfigStore that we can then inject into the admission webhook
that I have wired in there (nop for now, but demonstrating how it could work).
Idea being that you could then for a given image ask for all the authorities that
need to be validated. You can see what that config looks like in the
/pkg/apis/config/testdata/image-policies.yaml and the accompanying tests
in /pkg/apis/config/image_policies_test
I made sure that it works with both yaml/json.

While playing with this there's some questions that came to mind, so I'll take
those to the document.

Hope is that we get enough pieces in place so that we can agree on the major
moving pieces and how they fit together and enough testing in place that
we can start sharding up the work more efficiently and in more focused areas.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Add placeholder configmap with an example.
Rename to be consistent with the other cm. image-policies => config-image-policies

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Address lint.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Use namespaced sharedinformerfactory so that we have the right permissions.
Ran manual tests validating that things are working, when I remove
things from the configmap, things are patched back in (after the global
resync is triggered).

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Check error, duh.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

* Just trying to remove the files that verify-codegen is complaining.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants