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.

Sign up for GitHub

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

Jump to bottom

Fix aad support in kubectl for sovereign cloud #72143

Merged
merged 1 commit into from
Dec 20, 2018
Merged

Fix aad support in kubectl for sovereign cloud #72143

merged 1 commit into from
Dec 20, 2018

Conversation

karataliu
Copy link
Contributor

@karataliu karataliu commented Dec 18, 2018 •
edited

What type of PR is this?
/kind bug

What this PR does / why we need it:
This PR fix 2 issues

  1. Store environment name in config file
  2. Regard environment name when refreshing token

Which issue(s) this PR fixes:
Fixes #72227

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Fix AAD support for Azure sovereign cloud in kubectl

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. 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-priority Indicates a PR lacks a `priority/foo` label and requires one. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 18, 2018
@karataliu
Copy link
Contributor Author

/test pull-kubernetes-godeps

cc @andyzhangx

@andyzhangx
Copy link
Member

/sig azure

andyzhangx
andyzhangx reviewed Dec 18, 2018
View reviewed changes
Copy link
Member

@andyzhangx andyzhangx left a comment

Choose a reason for hiding this comment

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

this PR can only make into k8s v1.14 which is around March next year, are you planning to cherry-pick into old releases? if yes, pls do consider compatibility issue.

staging/src/k8s.io/client-go/plugin/pkg/client/auth/azure/azure.go Show resolved Hide resolved
@karataliu
Copy link
Contributor Author

/retest

1 similar comment
@karataliu
Copy link
Contributor Author

/retest

andyzhangx
andyzhangx approved these changes Dec 20, 2018
View reviewed changes
Copy link
Member

@andyzhangx andyzhangx left a comment

Choose a reason for hiding this comment

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

/lgtm
pls add an issue if you want to cherry-pick

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 20, 2018
@andyzhangx
Copy link
Member

/priority important-soon

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Dec 20, 2018
@karataliu
Copy link
Contributor Author

ping @lavalamp @mikedanese would you please take a look?

@mikedanese
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karataliu, 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2018
@k8s-ci-robot k8s-ci-robot merged commit c3e60b6 into kubernetes:master Dec 20, 2018
@karataliu karataliu deleted the kubectl.aad branch December 20, 2018 09:03
@fedebongio
Copy link
Contributor

/cc @apelisse

This was referenced Mar 21, 2019
Merged
Closed
Closed
k8s-ci-robot added a commit that referenced this pull request Apr 4, 2019
@k8s-ci-robot
Merge pull request #75532 from andyzhangx/automated-cherry-pick-of-#7…
73d746b 
…2143-upstream-release-1.13

Automated cherry pick of #72143: Fix aad support in kubectl for sovereign cloud
@feiskyer feiskyer added this to In progress in Provider Azure via automation Apr 17, 2019
@feiskyer feiskyer moved this from In progress to Done in Provider Azure Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andyzhangx andyzhangx andyzhangx approved these changes

@lavalamp lavalamp Awaiting requested review from lavalamp

@mikedanese mikedanese Awaiting requested review from mikedanese

@apelisse apelisse Awaiting requested review from apelisse

Assignees

@andyzhangx andyzhangx

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
Provider Azure
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AAD support in kubectl does not work well for sovereign cloud
5 participants
@karataliu @andyzhangx @mikedanese @k8s-ci-robot @fedebongio