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

[v13] Bump github.com/sigstore/cosign/v2 to 2.2.1 in /integrations/kube-agent-updater #34462

Merged
merged 5 commits into from
Nov 13, 2023

Conversation

jentfoo
Copy link
Contributor

@jentfoo jentfoo commented Nov 10, 2023

v13 Backport of #34428

Bumps [github.com/sigstore/cosign/v2](https://github.com/sigstore/cosign) from 2.0.1-0.20230228132138-2f1ec646de33 to 2.2.1.
- [Release notes](https://github.com/sigstore/cosign/releases)
- [Changelog](https://github.com/sigstore/cosign/blob/main/CHANGELOG.md)
- [Commits](https://github.com/sigstore/cosign/commits/v2.2.1)

---
updated-dependencies:
- dependency-name: github.com/sigstore/cosign/v2
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
@jentfoo jentfoo added go Issues related to Go builds/tooling dependencies Pull requests that update a dependency file no-changelog Indicates that a PR does not require a changelog entry labels Nov 10, 2023
@jentfoo jentfoo self-assigned this Nov 10, 2023
golang.org/x/mod v0.10.0
k8s.io/api v0.26.1
k8s.io/apimachinery v0.26.1
github.com/sigstore/cosign/v2 v2.2.1
Copy link
Contributor

@codingllama codingllama Nov 10, 2023

Choose a reason for hiding this comment

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

Note that this is a 2 minors jump.

@hugoShaka
Copy link
Contributor

This PR updates a lot of dependencies, are we really forced to update the kube libraries versions? I'm asking because this could break compatibility with old Kubernetes versions and I'd rather not bump the kube minor version if we can avoid it.

@jentfoo
Copy link
Contributor Author

jentfoo commented Nov 10, 2023

We do need this update in order to address CVE-2023-46737. Unfortunately cosign only released the fix as part of 2.2.1, so I don't see any older versions we can patch to.

@jentfoo
Copy link
Contributor Author

jentfoo commented Nov 13, 2023

@codingllama and @hugoShaka can you help me with this update? Do the failures here look familiar at all? I have looked for what change may be missing, but it's not obvious to me. v14 has been merged, so comparing the differences there may be useful. Thank you!

@hugoShaka
Copy link
Contributor

can you help me with this update? Do the failures here look familiar at all? I have looked for what change may be missing, but it's not obvious to me. v14 has been merged, so comparing the differences there may be useful. Thank you!

According to the linter logs

could not import sigs.k8s.io/controller-runtime/pkg/cache (-:
# sigs.k8s.io/controller-runtime/pkg/cache
  /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.4/pkg/cache/multi_namespace_cache.go:308:9:
cannot use handles (variable of type map[string]"k8s.io/client-go/tools/cache".ResourceEventandlerRegistration)
as "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration value
in return statement: map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration
does not implement "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration (missing method HasSynced)

This suggests two incompatible libraries are used together: one does not properly implement the interface used by the other (missing method HasSynced). Based on the logs I suspect the incompatibility happens between sigs.k8s.io/controller-runtime/pkg/cache and k8s.io/client-go/tools/cache.

I'm not a go dependency expert so I cannot advise you on the magic commands to run to fix the broken dependencies. Maybe @codingllama will know a cool trick to fix this.

@codingllama
Copy link
Contributor

A quick build attempt tells me the module needs tidying. After go mod tidy, it looks like controller-runtime@v0.14.x doesn't build with the new dependencies:

$ go build ./...
 
# sigs.k8s.io/controller-runtime/pkg/cache
/Users/alan/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.4/pkg/cache/multi_namespace_cache.go:308:9: cannot use handles (variable of type map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration) as "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration value in return statement: map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration does not implement "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration (missing method HasSynced)
/Users/alan/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.4/pkg/cache/multi_namespace_cache.go:321:9: cannot use handles (variable of type map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration) as "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration value in return statement: map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration does not implement "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration (missing method HasSynced)
/Users/alan/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.4/pkg/cache/multi_namespace_cache.go:326:17: impossible type assertion: h.(map[string]toolscache.ResourceEventHandlerRegistration)
	map[string]"k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration does not implement "k8s.io/client-go/tools/cache".ResourceEventHandlerRegistration (missing method HasSynced)

Updating controller-runtime itself brings a whole new set of breakages. k8s modules can be pretty finicky to update. I can't say right now what would work, this would take me a while to fix.

@codingllama
Copy link
Contributor

A quick blame shows me #31712, which we may need to backport along with this one.

@jentfoo
Copy link
Contributor Author

jentfoo commented Nov 13, 2023

@codingllama I looked at that PR, however I don't believe it's necessary since the controller-runtime is not being updated as part of this. However I did not test it.

@codingllama
Copy link
Contributor

I don't think you'll get this to build without updating the controller.

* adapt code to deal with controller-runtime breaking changes

* bump controller-runtime to v0.16.1
@jentfoo
Copy link
Contributor Author

jentfoo commented Nov 13, 2023

I added the changes from #27198 and #31712. Both are necessary to update the controller-runtime.

@jentfoo
Copy link
Contributor Author

jentfoo commented Nov 13, 2023

@hugoShaka With the runtime update this build appears to be passing. Can you re-review this PR and let me know if you feel there are significant compatibility concerns that we need to consider?

@hugoShaka
Copy link
Contributor

Looks good to me

@jentfoo jentfoo added this pull request to the merge queue Nov 13, 2023
Merged via the queue into branch/v13 with commit 8c0171e Nov 13, 2023
25 checks passed
@jentfoo jentfoo deleted the jent/cosign_update-v13 branch November 13, 2023 19:48
This was referenced Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport dependencies Pull requests that update a dependency file go Issues related to Go builds/tooling no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants