confidential-containers / cloud-api-adaptor

Ability to create Kata pods using cloud provider APIs aka the peer-pods approach
Apache License 2.0
48 stars 88 forks source link

golang: upgrade to 1.22.7 #2030

Closed bpradipt closed 2 months ago

bpradipt commented 2 months ago

Fixes Vulnerability #1: GO-2024-3106

bpradipt commented 2 months ago

I think you missed:

Were you planning to follow up with a separate PR to bump:

I missed the READMEs. Thanks for the pointers. Will update it.

bpradipt commented 2 months ago

@stevenhorsman should I move the changes dependent on the golang fedora image version into a separate PR?

stevenhorsman commented 2 months ago

@stevenhorsman should I move the changes dependent on the golang fedora image version into a separate PR?

We've had two different approaches for this:

  1. Have two PRs; wait for the golang fedora image to get merged in one and built then have the second one that uses it. This has the advantage of everything staying green
  2. Just do it all in one and manually re-run the builds that fail. This has the advantage of needing one less review cycle and as reviewers are hard to find recently, we've been doing this approach

I think given this is a major golang bump maybe approach 1 is safer this time?

bpradipt commented 2 months ago

Created a separate PR to update the fedora golang image - https://github.com/confidential-containers/cloud-api-adaptor/pull/2031 Once 2031 is merged, will update this PR

stevenhorsman commented 2 months ago

https://github.com/confidential-containers/cloud-api-adaptor/actions/runs/10771064012 shows that the image has been built now.

bpradipt commented 2 months ago

The e2 tests are timing out. I do see pod creation tests successfully executed. Maybe the timeout for the e2e tests could be increased. @stevenhorsman @mkulke should I merge this PR?

stevenhorsman commented 2 months ago

The e2 tests are timing out. I do see pod creation tests successfully executed. Maybe the timeout for the e2e tests could be increased. @stevenhorsman @mkulke should I merge this PR?

I think that's fine. I was going to say that the daily e2e runs haven't been working, but this morning's passed. I think we merge it and then I'll try and take a look at the test failures when I get a chance. From an initial glance it looks like the TestLibvirtCreateNginxDeployment is flakey and when it fails we are likely to trigger the timeout, but I'll try and get some more concrete info on this. Maybe that test is a candidate to only run on nightly and not PRs?

bpradipt commented 2 months ago

TestLibvirtCreateNginxDeployment is flakey and when it fails we are likely to trigger the timeout, but I'll try and get some more concrete info on this. Maybe that test is a candidate to only run on nightly and not PRs?

I'll also take a look at this test. Let's discuss this in our next community interlock. Probably by that time we'll have more insights on the flakiness.