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

Don't prompt before switching over to service account on Sandbox #3908

Merged

Conversation

datho7561
Copy link
Collaborator

Per our discussion, this makes the experience smoother

Signed-off-by: David Thompson davthomp@redhat.com

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (da60441) 32.37% compared to head (9d433c5) 45.28%.
Report is 56 commits behind head on main.

❗ Current head 9d433c5 differs from pull request most recent head a79f7ef. Consider uploading reports for the commit a79f7ef to get more accurate results

Files Patch % Lines
src/openshift/cluster.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3908       +/-   ##
===========================================
+ Coverage   32.37%   45.28%   +12.91%     
===========================================
  Files          85       85               
  Lines        6505     6525       +20     
  Branches     1349     1347        -2     
===========================================
+ Hits         2106     2955      +849     
+ Misses       4399     3570      -829     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@vrubezhny vrubezhny left a comment

Choose a reason for hiding this comment

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

It happens that if I'm logged out and trying to login to sandbox using a provided token, I get the following error:
Screenshot from 2024-02-16 12-02-48

in line 914 of installPipelineUserContext():

        const pipelineToken = Buffer.from(pipelineTokenSecret.data.token, 'base64').toString();

... because pipelineTokenSecret is undefined. (I've taken a look on gathered secrets - there is really no a secret which matadata.name is starting with pipeline-token).

As result I'm logged in, but still using the provided token, not the service account one

@vrubezhny
Copy link
Contributor

(I've taken a look on gathered secrets - there is really no a secret which matadata.name is starting with pipeline-token).

It doesn't matter if I user this PR or main branch, currently the situation is quite the when I try to login to api-sandbox-m3-1530-p1-openshiftapps-com:6443 (created just today after my previous Developer Sandbox for Red Hat OpenShift account was expired).

So in any case we have to check if the service account token is really available on a cluster

@datho7561
Copy link
Collaborator Author

Which ServiceAccounts are available for you on the cluster? You should be able to find this in the OpenShift Console web site switching to "Administrator", then clicking on "User Management" > "ServiceAccounts" in the sidebar.

@vrubezhny
Copy link
Contributor

vrubezhny commented Feb 16, 2024

Which ServiceAccounts are available for you on the cluster? You should be able to find this in the OpenShift Console web site switching to "Administrator", then clicking on "User Management" > "ServiceAccounts" in the sidebar.

image

For example, the defaut one:

kind: ServiceAccount
apiVersion: v1
metadata:
  name: default
  namespace: vrubezhny-dev
  ...
secrets:
  - name: default-dockercfg-6grkh
..

its metadata.name is default-dockercfg-6grkh.
There is no any service account with metadata.name: pipeline-token

@datho7561
Copy link
Collaborator Author

hmm, so they removed the pipeline account, which was the one with the edit permissions. I'm now starting to wonder if this is intentionally fixing a security vulnerability. The 15 minute long token is a security mechanism that prevents someone from using a computer where someone forgot to log out to make changes to the cluster. Switching to the long-lived token breaks this mechanism, so I was surprised the cluster provided such an account by default.

@vrubezhny
Copy link
Contributor

Yes, looks reasonable.
But still I believe there should be a way for developers to setup an isolated non-production cluster and get an account with no any time limitation (for development purposes, and not for production). The other question is if the same cluster is used for the development and for the production - then it's probably not possible.

@datho7561
Copy link
Collaborator Author

@vrubezhny I'm going to modify this PR so that it doesn't install the token if there is no pipeline ServiceAccount. We should get this in for the release.

Per our discussion, this makes the experience smoother

Signed-off-by: David Thompson <davthomp@redhat.com>
@datho7561 datho7561 force-pushed the don-t-prompt-to-use-service-account branch from 9d433c5 to a79f7ef Compare February 21, 2024 14:14
@vrubezhny
Copy link
Contributor

@vrubezhny I'm going to modify this PR so that it doesn't install the token if there is no pipeline ServiceAccount. We should get this in for the release.

Any news on possibility to have Service Account available in future?

Copy link
Contributor

@vrubezhny vrubezhny 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 to me. Thanks!

@vrubezhny vrubezhny merged commit 84c8b61 into redhat-developer:main Feb 21, 2024
4 checks passed
@datho7561 datho7561 deleted the don-t-prompt-to-use-service-account branch February 21, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants