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

Bootstrap token refactor #77211

Merged
merged 4 commits into from Jun 24, 2019

Conversation

@dixudx
Copy link
Member

commented Apr 29, 2019

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
Bootstrap token related functions are spread every where and defined multiple times.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

none
@dixudx

This comment has been minimized.

@neolit123

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

@neolit123

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

/priority important-longterm

@dixudx dixudx force-pushed the dixudx:bootstrap_token_refactor branch 2 times, most recently from 8095545 to 4795040 May 4, 2019

@neolit123

This comment has been minimized.

Copy link
Member

commented May 5, 2019

/retest

@luxas

luxas approved these changes May 8, 2019

Copy link
Member

left a comment

/approve
Thanks!

@neolit123

This comment has been minimized.

Copy link
Member

commented May 8, 2019

/assign @lavalamp @mikedanese
PTAL, thanks.

@dixudx dixudx force-pushed the dixudx:bootstrap_token_refactor branch from 7708c93 to 48d5d31 May 9, 2019

@timothysc

This comment has been minimized.

Copy link
Member

commented May 13, 2019

This is squarely on @kubernetes/sig-api-machinery-pr-reviews now.
/cc @sttts

@k8s-ci-robot k8s-ci-robot requested a review from sttts May 13, 2019

@dixudx

This comment has been minimized.

Copy link
Member Author

commented May 28, 2019

ping @kubernetes/sig-api-machinery-pr-reviews for review and approval.

@timothysc

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

/assign @liggitt

quid pro puo

@timothysc timothysc added this to the v1.16 milestone Jun 18, 2019

@mikedanese
Copy link
Member

left a comment

Controller and authenticator changes look fine to me. Added a couple nits.

@dixudx dixudx force-pushed the dixudx:bootstrap_token_refactor branch from 48d5d31 to 9e2c954 Jun 20, 2019

dixudx added some commits Apr 29, 2019

@dixudx dixudx force-pushed the dixudx:bootstrap_token_refactor branch from 9e2c954 to de8c6de Jun 20, 2019

@dixudx

This comment has been minimized.

Copy link
Member Author

commented Jun 20, 2019

/retest

@mikedanese

This comment has been minimized.

Copy link
Member

commented Jun 20, 2019

looks good
/approve

@dixudx

This comment has been minimized.

Copy link
Member Author

commented Jun 20, 2019

ping @lavalamp @sttts for approval. Thanks.

@@ -83,7 +84,7 @@ func encodeTokenSecretData(token *BootstrapToken, now time.Time) map[string][]by
// BootstrapTokenFromSecret returns a BootstrapToken object from the given Secret
func BootstrapTokenFromSecret(secret *v1.Secret) (*BootstrapToken, error) {
// Get the Token ID field from the Secret data
tokenID := getSecretString(secret, bootstrapapi.BootstrapTokenIDKey)
tokenID := bootstrapsecretutil.GetData(secret, bootstrapapi.BootstrapTokenIDKey)

This comment has been minimized.

Copy link
@liggitt

liggitt Jun 22, 2019

Member

all the GetData calls can be replaced with secret.Data[...keyname...]. If Data is nil or does not contain the specified key, that evaluates to the empty string

@liggitt

This comment has been minimized.

Copy link
Member

commented Jun 22, 2019

/approve

@timothysc
Copy link
Member

left a comment

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Jun 24, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dixudx, liggitt, luxas, mikedanese, timothysc

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

@neolit123

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 6f0f62b into kubernetes:master Jun 24, 2019

18 of 23 checks passed

pull-kubernetes-e2e-gce Job triggered.
Details
pull-kubernetes-e2e-gce-100-performance Job triggered.
Details
pull-kubernetes-integration Job triggered.
Details
pull-kubernetes-kubemark-e2e-gce-big Job triggered.
Details
pull-kubernetes-verify Job triggered.
Details
cla/linuxfoundation dixudx authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-publishing-bot-validate Job succeeded.
Details
tide In merge pool.
Details
@luxas
Copy link
Member

left a comment

Thanks @dixudx!

@dixudx dixudx deleted the dixudx:bootstrap_token_refactor branch Jun 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.