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

Experimental: Set the Tanzu Hub endpoint when creating tanzu context #734

Merged

Conversation

anujc25
Copy link
Contributor

@anujc25 anujc25 commented Apr 16, 2024

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

$ tz login --staging --endpoint https://api.tanzu-dev.cloud.vmware.com
[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=qvaHqJ1OM_170c3Bf5AP6ZkSbSIet8OiqX1RtPWMcx8&code_challenge_method=S256&redirect_uri=http%3A%2F%2F127.0.0.1%3A56866%2Fcallback&response_type=code&state=e9d6d53b690d4539fb13ee975bfe7014

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


[ok] Successfully logged into 'TAP pre-integration' organization and created a tanzu context
[i] Fetching recommended plugins for active context 'TAP_pre-integration-staging-d03c5c97'...
[ok] No recommended plugins found.
$ cat ~/.config/tanzu/config-ng.yaml
...
    - name: TAP_pre-integration-staging-d03c5c97
      target: tanzu
      contextType: tanzu
      globalOpts:
        endpoint: https://api.tanzu-dev.cloud.vmware.com
        auth:
            issuer: https://console-stg.cloud.vmware.com/csp/gateway/am/api
            userName: anujc
            permissions:
                - external/39721d32-3962-4a75-83d9-9b3dae23c39d/tap:viewer
                - external/39721d32-3962-4a75-83d9-9b3dae23c39d/tap:admin
                - csp:org_member
                - external/39721d32-3962-4a75-83d9-9b3dae23c39d/tap:member
            accessToken: ...
            IDToken: ...
            refresh_token: ...
            expiration: 2024-04-23T00:08:46.392013-07:00
            type: id-token
      clusterOpts:
        endpoint: https://api.tanzu-dev.cloud.vmware.com/org/ae93ebb4-a249-4553-aa1e-c87c4b7f75e5
        path: /Users/anujc/.kube/config
        context: tanzu-cli-TAP_pre-integration-staging-d03c5c97
      discoverySources: []
      additionalMetadata:
        tanzuHubEndpoint: https://be.symphony-dev.com/hub
        tanzuMissionControlEndpoint: https://tmc.tanzu-dev.cloud.vmware.com
        tanzuOrgID: ae93ebb4-a249-4553-aa1e-c87c4b7f75e5
        tanzuOrgName: TAP pre-integration
...

Release note

Experimental: Configure the Tanzu Hub endpoint when creating the `tanzu` context

Additional information

Special notes for your reviewer

@anujc25 anujc25 requested a review from a team as a code owner April 16, 2024 19:24
@anujc25 anujc25 marked this pull request as draft April 16, 2024 19:25
@anujc25 anujc25 force-pushed the set-tanzu-hub-endpoint-in-context branch 2 times, most recently from e1f5777 to 01765fc Compare April 23, 2024 06:19
@anujc25 anujc25 changed the title Draft: Set the Tanzu Hub endpoint when creating tanzu context Experimental: Set the Tanzu Hub endpoint when creating tanzu context Apr 23, 2024
@anujc25 anujc25 marked this pull request as ready for review April 23, 2024 06:31
@anujc25 anujc25 force-pushed the set-tanzu-hub-endpoint-in-context branch 3 times, most recently from ec5baff to 03c7b3b Compare April 23, 2024 06:36
go.mod Outdated Show resolved Hide resolved
if err != nil {
log.V(7).Infof("unable to get Tanzu Hub endpoint. Error: %v", err.Error())
} else {
c.AdditionalMetadata[config.TanzuHubEndpointKey] = tanzuHubEndpoint
Copy link
Contributor

Choose a reason for hiding this comment

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

This sets a new field in the metadata but in the call below updateTanzuContextMetadata I think we completely overwrite the metadata with the one from the existing context and we loose the tanzu hub field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amm. We only overwrite the metadata if the same context already exists right? So, will this be an issue for new contexts?

Copy link
Contributor

Choose a reason for hiding this comment

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

can we move this to after L666?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point @anujc25. Could the endpoint change on a re-login?

Upgrading an existing context when we first release this feature would be impacted but maybe that’s not a big deal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't see the further discussion on this thread. But I have updated the PR to account for re-login with the recent change.

}

for _, s := range services.ServicesList {
if s.ProductIdentifier == "TANZU-SAAS" && strings.Contains(s.DisplayName, "Tanzu Application Platform") { // TODO: Can this be improved to use some unique id?
Copy link
Contributor

@vuil vuil May 1, 2024

Choose a reason for hiding this comment

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

Not blocking, but maybe as a followup:
This looks like a check that could use some flexibility.
That is, if any of the string patterns could be subjected to change. Perhaps using a default set now, but having it be updatable via Central config could be an option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I will create a follow up PR to use Central Configuration Option to fetch this details.

if err != nil {
log.V(7).Infof("unable to get Tanzu Hub endpoint. Error: %v", err.Error())
} else {
c.AdditionalMetadata[config.TanzuHubEndpointKey] = tanzuHubEndpoint
Copy link
Contributor

Choose a reason for hiding this comment

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

can we move this to after L666?

Copy link
Contributor

@vuil vuil left a comment

Choose a reason for hiding this comment

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

lgtm, modulo the go dep update

@anujc25 anujc25 force-pushed the set-tanzu-hub-endpoint-in-context branch from 8282cdd to 47752df Compare May 1, 2024 22:42
@anujc25 anujc25 merged commit 790c57b into vmware-tanzu:main May 1, 2024
7 checks passed
vuil pushed a commit to vuil/tanzu-cli that referenced this pull request May 1, 2024
vmware-tanzu#734)

* Fetch Tanzu Hub endpoint from CSP when creating the `tanzu` context
vuil pushed a commit that referenced this pull request May 1, 2024
#734)

* Fetch Tanzu Hub endpoint from CSP when creating the `tanzu` context
@marckhouzam marckhouzam added this to the v1.3.0 milestone May 2, 2024
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.

None yet

4 participants