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

Introduce a regular patch release schedule for CA #5589

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

MaciekPytel
Copy link
Contributor

No description provided.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 10, 2023
@k8s-ci-robot k8s-ci-robot requested a review from x13n March 10, 2023 15:16
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 10, 2023
@MaciekPytel
Copy link
Contributor Author

@gjtempleton @towca @x13n I wrote down the proposal I discussed with you earlier on. I assigned release dates randomly, please suggest changes if you plan to be unavailable during your shift.

/hold
To get all the lgtms.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 10, 2023
@towca
Copy link
Collaborator

towca commented Mar 10, 2023

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 10, 2023
@x13n
Copy link
Member

x13n commented Mar 10, 2023

/lgtm

That's a bit outside of the scope of what you're adding, but I think that the new section should also include info about which minors actually are in scope for getting patched at all.

@MaciekPytel
Copy link
Contributor Author

@x13n I think it already does - "Cluster Autoscaler releases patches for versions corresponding to currently
supported Kubernetes versions". The link is updated and contains a list of versions.

Copy link
Contributor

@elmiko elmiko left a comment

Choose a reason for hiding this comment

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

i think this is nice, thanks @MaciekPytel
+1

@gjtempleton
Copy link
Member

/lgtm

@aaron-trout
Copy link

Can we describe in the process what is in-scope / out-of-scope for patch releases?

I joined the sig-autoscaling channel/call recently to look at regular rebuilds/release for security reasons (vulnerabilities in the existing images) and this seems to go a long way towards fixing that already. I'm wondering if it's reasonable to also update go dependencies on the release branches when creating these patch releases (at least for those with HIGH+ CVEs maybe?)

@MaciekPytel
Copy link
Contributor Author

MaciekPytel commented Mar 13, 2023

@aaron-trout That's a good point. A patch release would always be built using the newest base image, so it should address any vulnerabilities in the images. We could argue if 2-month latency is good enough, but we explicitly say additional releases may happen in response to vulnerabilities and if a serious vulnerability is escalated I'd imagine we'd be open to building a release in response (best escalation channel is probably via sig-meeting, either by participating or just adding to agenda).

Dependencies are more tricky - historically we haven't bumped them on patch releases, unless there was some specific bug that required us to do so. And vast majority of our dependencies are transitive dependencies of kubernetes, which we don't monitor directly. I don't think we want to get into the business of checking all those deps we're not even familiar with for vulns and even less so into solving any conflicts between kubernetes transitive deps and kubernetes itself.
That being said I'd imagine kubernetes itself is much better at managing this than we're (they have a sig dedicated to security after all), so maybe committing to bumping k8s to the latest patch version (which in turn will bump any transitive deps) on any patch release would be a reasonable middle ground here?

@x13n @towca @gjtempleton thoughts?

@x13n
Copy link
Member

x13n commented Mar 13, 2023

@x13n I think it already does - "Cluster Autoscaler releases patches for versions corresponding to currently supported Kubernetes versions". The link is updated and contains a list of versions.

Of course I missed the first sentence of the diff 🤦 Thanks, just lgtm from me then!

@x13n
Copy link
Member

x13n commented Mar 13, 2023

Dependencies are more tricky - historically we haven't bumped them on patch releases, unless there was some specific bug that required us to do so. And vast majority of our dependencies are transitive dependencies of kubernetes, which we don't monitor directly. I don't think we want to get into the business of checking all those deps we're not even familiar with for vulns and even less so into solving any conflicts between kubernetes transitive deps and kubernetes itself. That being said I'd imagine kubernetes itself is much better at managing this than we're (they have a sig dedicated to security after all), so maybe committing to bumping k8s to the latest patch version (which in turn will bump any transitive deps) on any patch release would be a reasonable middle ground here?

I think bumping k8s deps should be good enough unless someone explicitly reaches out about a critical vulnerability in some non-k8s deps in which case we should also ensure the fix is included.

@aaron-trout
Copy link

Sounds good. It may be worth doing this (checking for vulns and updating transitive deps) as a one off when implementing this new release policy since many of the images have HIGH / CRITICAL CVEs currently. For example here is a scan of the 1.24.0 image using trivy:

Screenshot 2023-03-06 at 18 02 24

I'm also happy to do some local builds off the release branches and scan the resulting images for comparison if that is helpful? I can raise some PRs to the release branches to attempt to fix any found issues.

Copy link
Contributor

@mwielgus mwielgus left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@mwielgus mwielgus removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 16, 2023
@k8s-ci-robot k8s-ci-robot merged commit 7bb1f0f into kubernetes:master Mar 16, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MaciekPytel, mwielgus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cluster-autoscaler cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants