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

Add support for apb catalog relist #79

Merged
merged 6 commits into from
Jul 10, 2018

Conversation

dymurray
Copy link
Contributor

@dymurray dymurray commented Jul 9, 2018

Includes small error check for current-context gathering.

Note: I would like to come back to this to use the dynamic go client to grab these resources but this is clean for now.

@dymurray
Copy link
Contributor Author

Fixes #56

@djwhatle
Copy link
Contributor

djwhatle commented Jul 10, 2018

@dymurray what do I need to do to make this work? I tried this:

$ apb catalog relist
WARN Failed to create a InternalClientSet: unable to load in-cluster configuration, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined. 
ERRO Failed to get relist response. Expected status 200, got: 403 

@djwhatle djwhatle self-requested a review July 10, 2018 15:24
@djwhatle
Copy link
Contributor

Added more detailed error messages and suggested actions:

relist attempt without a bearer token

[dwhatley@precision-t apb]$ oc login -u system:admin
Logged into [...] as "system:admin" using existing credentials.
[...]
Using project "default".
[dwhatley@precision-t apb]$ apb catalog relist
ERRO Bearer token not found for current 'oc' user. Log in as a different user and retry. 
INFO View your current token with 'oc whoami -t'  
INFO Some users don't have a token, including 'system:admin' 

relist attempt without privileges to view routes

[dwhatley@precision-t apb]$ apb catalog relist
ERRO Bad relist response. Expected status 200, got: 403
{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"clusterservicebrokers.servicecatalog.k8s.io \"openshift-automation-service-broker\" is forbidden: User \"apb-user\" cannot get clusterservicebrokers.servicecatalog.k8s.io at the cluster scope: User \"apb-user\" cannot get clusterservicebrokers.servicecatalog.k8s.io at the cluster scope","reason":"Forbidden","details":{"name":"openshift-automation-service-broker","group":"servicecatalog.k8s.io","kind":"clusterservicebrokers"},"code":403}
 
ERRO Current 'oc' user unable to get 'clusterservicebrokers'. Try again with a more privileged user.
 
INFO Administrators can grant 'cluster-admin' privileges with: 
INFO 'oc adm policy add-cluster-role-to-user cluster-admin <oc-user>' 

@djwhatle
Copy link
Contributor

@dymurray in catasb env I found that I was unable to relist without manually specifying the name of my broker, which is expected, but we should put a PR into catasb to fix this (change ansible-service-broker to openshift-ansible-service-broker).

Short: "apb is a tool to manage ServiceBundle images",
Long: `ServiceBundles are images that represent lifecycle components
in that they contain all of the orchestration logic to manage
Short: "apb is a tool to manage Ansible Playbook Bundle images",
Copy link
Contributor

@djwhatle djwhatle Jul 10, 2018

Choose a reason for hiding this comment

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

Not really related to your PR, but the long summary here feels overly detailed to be in the bare apb command help-text. Think we should consider stripping this down to just the short version.

Copy link
Contributor

@djwhatle djwhatle left a comment

Choose a reason for hiding this comment

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

Will be good to get this ported over to client-go so that we don't need a bearer token in the future, but works well enough for now since it's equivalent to previous tool.

$ apb catalog relist --name ansible-service-broker
Successfully relisted OpenShift Service Catalog for [ansible-service-broker]

@dymurray dymurray merged commit 272e4e2 into automationbroker:master Jul 10, 2018
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