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

Ignore command PreRun logic for tanzu context get-token command #754

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

prkalle
Copy link
Contributor

@prkalle prkalle commented Apr 29, 2024

What this PR does / why we need it

This PR would ignore command PreRun logic for tanzu context get-token command.

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

Tested the kubectl command (which internally would call the tanzu context get-token command) after removing the CEIP and EULA opt-in status from the CLI config file. Verified that CLI binary without the fix get stuck and later with updated binary path , the CLI didn't get hang for prompt.

// removed the CEIP and EULA status form the config file.
❯ vim ~/.config/tanzu/config-ng.yaml

❯ kubectl api-resources
[x] : failed to get EULA status: prompt failed: interrupt
NAME   SHORTNAMES   APIVERSION   NAMESPACED   KIND
Unable to connect to the server: getting credentials: exec: executable tanzu failed with exit code 1

//updated the path of the tanzu binary in the exec plugin of the kubeconfig
❯ vim ~/.kube/config
- name: tanzu-cli-TAP_pre-integration-staging-d03c5c97-user
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1
      args:
      - context
      - get-token
      - TAP_pre-integration-staging-d03c5c97
      command: /Users/pkalle/projects/tanzu-cli/bin/tanzu
      env: []
      interactiveMode: IfAvailable
      provideClusterInfo: false



// now with updated CLI binary the login works

❯ kubectl api-resources
[i] Opening the browser window to complete the login
Log in by visiting this link:

    https://console-stg.cloud.vmware.com/csp/gateway/discovery?client_id=tanzu-cli-client-id&code_challenge=ND_rBKs_kxm_TPAyq-n4Qf3mzYGug2ceQuiz4TG5G40&code_challenge_method=S256&orgId=ae93ebb4-a249-4553-aa1e-c87c4b7f75e5&redirect_uri=http%3A%2F%2F127.0.0.1%3A56182%2Fcallback&response_type=code&state=63537b30798749af61469c7d21250095

    Optionally, paste your authorization code: [...]

NAME   SHORTNAMES   APIVERSION   NAMESPACED   KIND

Release note

Ignore command PreRun logic for the tanzu context get-token command 

Additional information

Special notes for your reviewer

@prkalle prkalle requested a review from a team as a code owner April 29, 2024 17:20
@prkalle prkalle force-pushed the fix/gettoken-prerun branch from 0091b1b to c7e3d58 Compare April 29, 2024 17:56
@marckhouzam marckhouzam added this to the v1.3.0 milestone Apr 29, 2024
@marckhouzam
Copy link
Contributor

Thanks @prkalle. Could you also add it to shouldSkipVersionCheck()? The version check logic could print that a new version is available and we should avoid triggering that from kubectl

Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

Looks good other than what Marc mentioned above.

@prkalle prkalle force-pushed the fix/gettoken-prerun branch from c7e3d58 to 06c0e78 Compare April 30, 2024 18:18
@prkalle
Copy link
Contributor Author

prkalle commented Apr 30, 2024

Could you also add it to shouldSkipVersionCheck()? The version check logic could print that a new version is available and we should avoid triggering that from kubectl

Thanks @marckhouzam. Added tanzu context get-token command to shouldSkipVersionCheck().

Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @prkalle

@prkalle prkalle merged commit 92c6b55 into vmware-tanzu:main Apr 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants