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

Use an explicit token to identify kubeapps cluster when required. #4591

Closed

Conversation

absoludity
Copy link
Contributor

Signed-off-by: Michael Nelson minelson@vmware.com

Description of the change

For discussion related to #4564 .

This PR aims to ensure that the context for an available package can always be resolved, even when there is no configured cluster name corresponding to the cluster on which Kubeapps is installed.

We need to test this as I suspect, if installing a package still requires fetching the AppRepository, that it would mean that the user credential must be valid to authenticate with the cluster on which Kubeapps is installed.
The user doesn't need any RBAC, only the ability to authenticate, since Kubeapps already includes RBAC to ensure that global repositories are readable by any authenticated user.

Also, if pinniped-proxy is being used, it won't currently handle this scenario (where the new token is used for the cluster on which Kubeapps is installed), but that will be easy to update. I'll add that depending on the discussion here.

Benefits

Possible drawbacks

Applicable issues

  • fixes #

Additional information

For discussion.

Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
@antgamdia antgamdia added the next-iteration Issues to be discussed in planning session label Aug 11, 2022
@netlify
Copy link

netlify bot commented Aug 11, 2022

Deploy Preview for kubeapps-dev canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 64c43df
🔍 Latest deploy log https://app.netlify.com/sites/kubeapps-dev/deploys/633ee545e21e5500082a563e

@antgamdia antgamdia marked this pull request as ready for review September 2, 2022 18:28
@antgamdia antgamdia marked this pull request as draft September 2, 2022 18:28
castelblanque added a commit that referenced this pull request Oct 26, 2022
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

### Description of the change

Adapted version of the outdated PR #4591. After removing kubeops, there
has been a refactor of places changes by the old PR.
It fixes the scenario in which the cluster where Kubeapps is installed
is not part of the configured list of clusters.
For example:

```yaml
clusters:
  - name: second-cluster
    domain: ...
    apiServiceURL: ....
    certificateAuthorityData: ...
    serviceToken: eyJhbGciOiJS...
```

For doing so, it uses a token to reference the Kubeapps cluster: `-`.
Also empty string `''` is accepted as cluster reference, for allowing
payloads in which there is no cluster specified in context.

Therefore, if a cluster string is empty or `-`, backend logic assumes
that the Kubeapps cluster will be used.

### Benefits

User can work in a multicluster setups in which the Kubeapps cluster is
not part of the allowed clusters.

### Possible drawbacks

N/A

### Applicable issues

- fixes #4564

### Additional information

Manual tests have been done with Helm plugin (only one working in
multicluster so far).
CI E2E test will be done in #4563

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
@antgamdia
Copy link
Contributor

Once #5566 was merged, can we close this one, @castelblanque ?

@castelblanque
Copy link
Collaborator

Indeed.
Closing as this PR has been superseeded by #5566.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-iteration Issues to be discussed in planning session
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants