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

Move credential provider code to staging/ #95775

Merged

Conversation

DangerOnTheRanger
Copy link
Contributor

@DangerOnTheRanger DangerOnTheRanger commented Oct 21, 2020

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

This PR moves the credential provider code that used to exist in config.go and provider.go in pkg/credentialprovider into staging/src/k8s.io/cloud-provider/credentialconfig, and distributes the GCP-specific code in pkg/credentialprovider/gcp that does not rely on pkg/credentialprovider into staging/src/k8s.io/legacy-cloud-providers/gce/gcpcredential. This way, when #94196 is merged, relevant GCP code can be cleanly shared between the legacy in-tree credential provision implementations and plugin-based out-of-tree implementations.

Which issue(s) this PR fixes:
none

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/blob/b7a365ec9a033a6208faf2d688b91f87c7338a31/keps/sig-cloud-provider/20191004-out-of-tree-credential-providers.md

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 21, 2020
@k8s-ci-robot
Copy link
Contributor

Welcome @DangerOnTheRanger!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 21, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @DangerOnTheRanger. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added area/cloudprovider area/kubelet sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 21, 2020
@cheftako
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 23, 2020
@DangerOnTheRanger DangerOnTheRanger changed the title [WIP] Move GCP credential provider code to staging/ [WIP] Move credential provider code to staging/ Oct 24, 2020
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/dependency Issues or PRs related to dependency changes and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 23, 2021
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 23, 2021
@DangerOnTheRanger
Copy link
Contributor Author

/test pull-kubernetes-e2e-kind

@DangerOnTheRanger
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce-ubuntu-containerd

@DangerOnTheRanger
Copy link
Contributor Author

/retest

@kcmartin
Copy link

kcmartin commented Mar 5, 2021

Hello! I am from the Bug Triage team! Friendly reminder that code freeze is starting March 9th, 2021 (less than 1 week from now). We want to ensure that each PR has a chance to be merged before code freeze.

As the PR is tagged for 1.21, is it still planned for this release?

@DangerOnTheRanger
Copy link
Contributor Author

Hello! I am from the Bug Triage team! Friendly reminder that code freeze is starting March 9th, 2021 (less than 1 week from now). We want to ensure that each PR has a chance to be merged before code freeze.

As the PR is tagged for 1.21, is it still planned for this release?

Yes, the PR has already undergone several reviews and shouldn't require any major modifications at this point in time. Thanks for reaching out!

@cheftako
Copy link
Member

cheftako commented Mar 9, 2021

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 9, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, DangerOnTheRanger, derekwaynecarr, dims, erictune, mikedanese

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

@cheftako
Copy link
Member

cheftako commented Mar 9, 2021

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 9, 2021
@fejta-bot
Copy link

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@DangerOnTheRanger
Copy link
Contributor Author

/retest

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Mar 9, 2021

@DangerOnTheRanger: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-publishing-bot-validate 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-publishing-bot-validate
pull-kubernetes-e2e-gce-network-proxy-grpc 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gce-network-proxy-grpc
pull-kubernetes-e2e-gce-iscsi-serial 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gce-iscsi-serial
pull-kubernetes-e2e-gce-iscsi 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gce-iscsi
pull-kubernetes-e2e-gce-storage-snapshot 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gce-storage-snapshot
pull-kubernetes-e2e-gce-storage-slow 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gce-storage-slow
pull-kubernetes-e2e-gce-alpha-features 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gce-alpha-features
pull-kubernetes-local-e2e 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-local-e2e
pull-kubernetes-files-remake 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-files-remake
pull-kubernetes-e2e-gci-gce-ipvs 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gci-gce-ipvs
pull-kubernetes-e2e-gce-csi-serial 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gce-csi-serial
pull-kubernetes-conformance-kind-ipv6-parallel 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-conformance-kind-ipv6-parallel
pull-kubernetes-e2e-gce-network-proxy-http-connect 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-e2e-gce-network-proxy-http-connect
pull-kubernetes-conformance-image-test 2696d5b5450b4a02b3d724bd431976191f71faf5 link /test pull-kubernetes-conformance-image-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@DangerOnTheRanger
Copy link
Contributor Author

/test pull-kubernetes-e2e-kind

@k8s-ci-robot k8s-ci-robot merged commit 45cf7e0 into kubernetes:master Mar 9, 2021
SIG Node PR Triage automation moved this from Waiting on Author to Done Mar 9, 2021
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/cloudprovider area/code-organization Issues or PRs related to kubernetes code organization area/dependency Issues or PRs related to dependency changes area/kubelet area/provider/gcp Issues or PRs related to gcp provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

Successfully merging this pull request may close these issues.

None yet