KEP-4193: Update milestone for v1.30 & clarify latest state #4470

munnerz · 2024-02-01T15:19:51Z

One-line PR description: Updating KEP milestone
Issue link: Bound service account token improvements #4193

liggitt · 2024-02-01T15:30:52Z

/lgtm
/approve

liggitt · 2024-02-01T15:31:19Z

/assign @deads2k
for PRR ack

enj

nit, LGTM

keps/sig-auth/4193-bound-service-account-token-improvements/README.md

…ADME.md Co-authored-by: Mo Khan <i@monis.app>

liggitt · 2024-02-05T17:34:12Z

/lgtm
/approve

soltysh

#prr-shadow

keps/sig-auth/4193-bound-service-account-token-improvements/README.md

soltysh · 2024-02-05T18:46:29Z

keps/sig-auth/4193-bound-service-account-token-improvements/README.md


 This JTI is then used for a further audit log lookup - namely, looking for the TokenRequest `create` call which contains
-the audit annotation with key `authentication.kubernetes.io/token-identifier` and the value set to that of the suspect token.
+the audit annotation with key `authentication.kubernetes.io/issued-credential-id` and the value set to that of the suspect token.

 This allows Bob to determine precisely who made the original request for this token, and (depending on the 'chain'
 above this token), allows Bob to recursively perform this lookup to find all involved parties that led to this token


In the Test Plan section, please:

Make sure to check the checkbox in the first paragraph.

Make sure to update the test coverage for modified packages in Unit tests.

Make sure to provide links for e2e and integration tests to testgrid.

I've gone through and done all these, though for (3), this whole area (bound service account token issuance) doesn't have any e2e or conformance tests as it is - it's all being tested via integration tests at the moment.

Does that mean I should do the same here (which is already complete), or is e2e/conformance testing for all changes a new requirement added since we added bound tokens in ~1.21-ish?

I've gone through and done all these, though for (3), this whole area (bound service account token issuance) doesn't have any e2e or conformance tests as it is - it's all being tested via integration tests at the moment.

Integration seems sufficient, but please make sure to link it in the appropriate section. About e2e I'll leave it up to you and auth approvers.

keps/sig-auth/4193-bound-service-account-token-improvements/README.md

soltysh · 2024-02-05T19:03:47Z

keps/sig-auth/4193-bound-service-account-token-improvements/README.md

+The `ServiceAccountTokenNodeBindingValidation` feature will graduate to beta one release earlier than `ServiceAccountTokenNodeBinding`
+to ensure a safe rollback from version N+1 to N (more info below in rollback considerations section).
+
+All other feature flags can be disabled without any unexpected adverse affects or coordination required.

 ###### How can this feature be enabled / disabled in a live cluster?


Further down in the doc, please:

Answer the question ###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?.

###### What are other known failure modes? should probably list the feature enablement ordering issue.

###### What steps should be taken if SLOs are not being met to determine the problem? needs an answer

Implementation history needs update

for (1), what kind of test should I actually perform at each stage of upgrade->downgrade->upgrade again? given this touches a subset of tokens that can be used to authenticate to an apiserver, is it sufficient to just test that these credentials are valid/invalid depending on the upgrade/downgrade (or enablement) status? Is simply enabling the gates and then disabling them again sufficient, or would you expect a full apiserver upgrade to be performed?

for (2), I've not included this as I've added an additional check into the apiserver to prevent startup if the wrong combination of gates is enabled (see #4470 (comment))

(3) and (4) have been completed

thanks!

for (1), what kind of test should I actually perform at each stage of upgrade->downgrade->upgrade again? given this touches a subset of tokens that can be used to authenticate to an apiserver, is it sufficient to just test that these credentials are valid/invalid depending on the upgrade/downgrade (or enablement) status? Is simply enabling the gates and then disabling them again sufficient, or would you expect a full apiserver upgrade to be performed?

yup, that sounds sufficient.

for (2), I've not included this as I've added an additional check into the apiserver to prevent startup if the wrong combination of gates is enabled (see #4470 (comment))

thank you!

(3) and (4) have been completed

thank you!

soltysh · 2024-02-06T15:26:23Z

#prr-shadow
so we're down to two rather minor bits:

upgrade description (KEP-4193: Update milestone for v1.30 & clarify latest state #4470 (comment))
integration links (KEP-4193: Update milestone for v1.30 & clarify latest state #4470 (comment))

munnerz · 2024-02-07T11:28:57Z

#prr-shadow so we're down to two rather minor bits:

upgrade description (KEP-4193: Update milestone for v1.30 & clarify latest state #4470 (comment))

integration links (KEP-4193: Update milestone for v1.30 & clarify latest state #4470 (comment))

thanks! i've added a description of upgrade/downgrade (or rather, feature enablement/disablement) as well as links to integration tests.

soltysh

#prr-shadow
the prr lgtm

deads2k · 2024-02-07T15:48:45Z

lgtm both for sig-auth and prr

/tide merge-method-squash
/lgtm
/approve

k8s-ci-robot · 2024-02-07T15:48:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, liggitt, munnerz

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:

~~keps/prod-readiness/OWNERS~~ [deads2k]
~~keps/sig-auth/OWNERS~~ [deads2k,liggitt]

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

Update KEP-4193 milestone for 1.30

ab65248

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/auth Categorizes an issue or PR as relevant to SIG Auth. labels Feb 1, 2024

k8s-ci-robot requested review from jeremyrickard and mikedanese February 1, 2024 15:20

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 1, 2024

munnerz mentioned this pull request Feb 1, 2024

Bound service account token improvements #4193

Open

25 tasks

k8s-ci-robot assigned liggitt Feb 1, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 1, 2024

k8s-ci-robot assigned deads2k Feb 1, 2024

KEP-4193: updates to reflect latest state of feature

a1c3dcd

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 5, 2024

munnerz changed the title ~~KEP-4193: Update milestone for 1.30~~ KEP-4193: Update milestone for v1.30 & clarify latest state Feb 5, 2024

enj reviewed Feb 5, 2024

View reviewed changes

keps/sig-auth/4193-bound-service-account-token-improvements/README.md Outdated Show resolved Hide resolved

Update keps/sig-auth/4193-bound-service-account-token-improvements/RE…

ab8d240

…ADME.md Co-authored-by: Mo Khan <i@monis.app>

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 5, 2024

soltysh reviewed Feb 5, 2024

View reviewed changes

update PRR & test plan contents

3a9799e

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed 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. labels Feb 6, 2024

add links to integration tests & upgrade/downgrade explanation

1ded674

k8s-ci-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 7, 2024

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 7, 2024

soltysh reviewed Feb 7, 2024

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 7, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 7, 2024

k8s-ci-robot merged commit e95fd98 into kubernetes:master Feb 7, 2024
4 checks passed

k8s-ci-robot added this to the v1.30 milestone Feb 7, 2024

munnerz deleted the 4193-update-milestone branch February 7, 2024 16:01

soltysh mentioned this pull request Apr 3, 2024

Add soltysh to prod-readiness-approvers #4566

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KEP-4193: Update milestone for v1.30 & clarify latest state #4470

KEP-4193: Update milestone for v1.30 & clarify latest state #4470

munnerz commented Feb 1, 2024

liggitt commented Feb 1, 2024

liggitt commented Feb 1, 2024

enj left a comment

liggitt commented Feb 5, 2024

soltysh left a comment

soltysh Feb 5, 2024

munnerz Feb 6, 2024

soltysh Feb 6, 2024

soltysh Feb 5, 2024

munnerz Feb 6, 2024

soltysh Feb 6, 2024

soltysh commented Feb 6, 2024

munnerz commented Feb 7, 2024

soltysh left a comment

deads2k commented Feb 7, 2024

k8s-ci-robot commented Feb 7, 2024

KEP-4193: Update milestone for v1.30 & clarify latest state #4470

KEP-4193: Update milestone for v1.30 & clarify latest state #4470

Conversation

munnerz commented Feb 1, 2024

liggitt commented Feb 1, 2024

liggitt commented Feb 1, 2024

enj left a comment

Choose a reason for hiding this comment

liggitt commented Feb 5, 2024

soltysh left a comment

Choose a reason for hiding this comment

soltysh Feb 5, 2024

Choose a reason for hiding this comment

munnerz Feb 6, 2024

Choose a reason for hiding this comment

soltysh Feb 6, 2024

Choose a reason for hiding this comment

soltysh Feb 5, 2024

Choose a reason for hiding this comment

munnerz Feb 6, 2024

Choose a reason for hiding this comment

soltysh Feb 6, 2024

Choose a reason for hiding this comment

soltysh commented Feb 6, 2024

munnerz commented Feb 7, 2024

soltysh left a comment

Choose a reason for hiding this comment

deads2k commented Feb 7, 2024

k8s-ci-robot commented Feb 7, 2024