Skip to content

Comments

Bug 1944581: Preserve AuthInfo when switching projects#692

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
feichashao:backplane-project
May 19, 2021
Merged

Bug 1944581: Preserve AuthInfo when switching projects#692
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
feichashao:backplane-project

Conversation

@feichashao
Copy link
Contributor

This is a possible fix for #647

Before this patch, if the openshift cluster is behind a cluster proxy (eg. kubectl proxy), and the authentication method is not token, oc project <project-name> will create a new user in kubeconfig, with blank info. This will lead to further oc commands return authentication error.

- name: system:serviceaccount:openshift-xxxx-xxxx:xxxxx
  user: {}

With this patch, when oc project <project-name> creating a new kubeconfig, it will preserve all the AuthInfo from RESTConfig, so that it won't break the customized settings.

- name: system:serviceaccount:openshift-xxxxx-xxxx/xxx-xxx-xxxx-xxxxx-xxxx-xx-xxxx-com:443
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1beta1
      args:
      - /home/xxx/.kube/xxx-token
      command: bash
      env:
      - name: OCM_CONFIG
        value: /home/xxx/.xxxx.xxxx.stg
      provideClusterInfo: false

clientcmdapi will handle the null pointer and null strings, so it won't break the existing oc login or oc project mechanism.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: feichashao
To complete the pull request process, please assign soltysh after the PR has been reviewed.
You can assign the PR to them by writing /assign @soltysh in a comment when ready.

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

Details 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

@soltysh
Copy link
Contributor

soltysh commented Jan 11, 2021

/retest

3 similar comments
@wanghaoran1988
Copy link
Member

/retest

@wanghaoran1988
Copy link
Member

/retest

@feichashao
Copy link
Contributor Author

/retest

@feichashao
Copy link
Contributor Author

The ci/prow/e2e-agnostic-cmd failure is pretty common accross other PRs, so i think it is not related to this PR change.

The first commit preserves Impersonate + Exec + tokenfile to kubeconfig when swithing projects.
f826dc2

The second commit remove the presevation of Impersonate, because users/automated-scripts might use oc --as=<someone> to switch project. If we put the Impersonate info into kubeconfig when switch projects, it is possible to break existing scripts.
d7fd77c

The oc command does not have options for Exec, tokenfile and AuthProvider, so it should be fine to preseve those info when swithing project, without breaking the existing behavior.

@feichashao
Copy link
Contributor Author

/retest

@feichashao
Copy link
Contributor Author

Testes passed. @soltysh could you take a look?

@soltysh soltysh changed the title Preserve AuthInfo when switching projects Bug 1944581: Preserve AuthInfo when switching projects May 18, 2021
@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels May 18, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 18, 2021

@feichashao: This pull request references Bugzilla bug 1944581, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @zhouying7780

Details

In response to this:

Bug 1944581: Preserve AuthInfo when switching projects

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.

@openshift-ci openshift-ci bot requested a review from zhouying7780 May 18, 2021 20:02
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

This looks good, please squash changes and I'll tag

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 18, 2021
@feichashao feichashao force-pushed the backplane-project branch from d7fd77c to 8062b92 Compare May 19, 2021 01:49
@cblecker
Copy link
Member

/retest

1 similar comment
@feichashao
Copy link
Contributor Author

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 19, 2021

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

Test name Commit Details Rerun command
ci/prow/e2e-metal-ipi-ovn-ipv6 8062b92 link /test e2e-metal-ipi-ovn-ipv6

Full PR test history. Your PR dashboard.

Details

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.

@feichashao
Copy link
Contributor Author

/retest

Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 19, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 19, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feichashao, soltysh

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

The pull request process is described here

Details 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

@openshift-merge-robot openshift-merge-robot merged commit 0099df7 into openshift:master May 19, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 19, 2021

@feichashao: All pull requests linked via external trackers have merged:

Bugzilla bug 1944581 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1944581: Preserve AuthInfo when switching projects

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.

@wanghaoran1988
Copy link
Member

/cherry-pick release-4.7

@openshift-cherrypick-robot

@wanghaoran1988: new pull request created: #832

Details

In response to this:

/cherry-pick release-4.7

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.

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. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants