print the related namespace when login via manager cluster #274
Conversation
openshift-ci
bot
added
the
approved
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #274 +/- ##
==========================================
- Coverage 46.91% 46.68% -0.23%
==========================================
Files 53 53
Lines 3568 3911 +343
==========================================
+ Hits 1674 1826 +152
- Misses 1633 1792 +159
- Partials 261 293 +32
|
cmd/ocm-backplane/login/login.go
Outdated
| var env string | ||
| if strings.Contains(bpURL, "integration") { | ||
| env = "integration" | ||
| } else if strings.Contains(bpURL, "stage") { | ||
| env = "staging" | ||
| } else { | ||
| env = "production" | ||
| } | ||
|
|
There was a problem hiding this comment.
This may expose all available RH env's to the public. You may able to get the env with ocm get /api/clusters_mgmt/v1/environment API or wait for OSD-19188 :)
There was a problem hiding this comment.
Yeah, I feel after the OSD-19188 there would be easier way to get it.
But I think even with the ocm get, the namespace prefix may not match the env name? like staging vs stage
There was a problem hiding this comment.
Yeah, we shouldn't be exposing any clue what our API URLs even look like. I wouldn't prefer to let this change in.
cmd/ocm-backplane/login/login.go
Outdated
| func listNamespaces(clusterID, clusterName, env string, isHypershift bool) []string { | ||
|
|
||
| klusterletPrefix := "klusterlet-" | ||
| hivePrefix := fmt.Sprintf("uhc-%s-", env) | ||
| hcpPrefix := fmt.Sprintf("ocm-%s-", env) | ||
|
|
||
| var nsList []string | ||
|
|
||
| if isHypershift { | ||
| nsList = []string{ | ||
| klusterletPrefix + clusterID, | ||
| hcpPrefix + clusterID, | ||
| hcpPrefix + clusterID + "-" + clusterName, | ||
| } | ||
| } else { | ||
| nsList = []string{ | ||
| hivePrefix + clusterID, | ||
| } | ||
| } | ||
|
|
||
| return nsList | ||
| } |
There was a problem hiding this comment.
What if we get the matching namespaces via bp-api endpoint ? basically oc get ns -A| grep {cluster-internal-id}
There was a problem hiding this comment.
I just think to send less api request, if we do the oc get ns we even do not need to detect the env
There was a problem hiding this comment.
The namespace pattern is fixed so it should be ok to print them directly.
There was a problem hiding this comment.
Is this pattern unique to the FedRamp stack too?
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
|
/hold Hold this for now, as we are waiting for an implementation which can get the env info automatically |
openshift-ci
bot
added
the
do-not-merge/hold
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bmeng, samanthajayasinghe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@bmeng: all tests passed! Full PR test history. Your PR dashboard. 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. |
What type of PR is this?
(feature)
What this PR does / Why we need it?
It prints the cluster related namespaces when login with the
--manageroption, since user may like to investigate the resources withinWhich Jira/Github issue(s) does this PR fix?
Resolves #OSD-19081
Special notes for your reviewer
Pre-checks (if applicable)