cloudfoundry / cli

The official command line client for Cloud Foundry
https://docs.cloudfoundry.org/cf-cli
Apache License 2.0
1.75k stars 928 forks source link

Provide better error message if CF deployment fails due to missing authorization #1523

Open fwilhe opened 5 years ago

fwilhe commented 5 years ago

Context

I'm working on SAP S/4HANA Cloud SDK Pipeline where we make use of the cf cli to push applications to SAP Cloud Platform. We use the cli via a Docker image.

Observation

If you configure a valid user for deployment that is not a member of the target subaccount, the error message is not descriptive of the misconfiguration.

Log excerpt:

Error finding org myorg
Organization myorg not found
...
Could not load apps in space, are you logged in? - No org and space targeted, use 'cf target -o ORG -s SPACE' to target an org and space

Improvement

I think the message as it is now is misleading in this case, and will make it quite hard for the user to find the error, which is very frustrating. An easy solution could be to mention this (and maybe other) possible error conditions in the message. The better, but more complex solution would be to include such information dynamically in the error message.

This is where the message comes from https://github.com/cloudfoundry/cli/blob/b6a3695a5db06d6628428067e0a78e63041ea1ca/cf/requirements/targeted_space.go#L24

cf-gitbot commented 5 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/162608501

The labels on this github issue will be updated when the story is started.

tjvman commented 5 years ago

Could you tell us the exact command (or series of commands) that you're running that's causing this failure? Also, I'm not sure what you mean by "subaccount"; could you clairfy that?

At a glance, though, this seems like it may be a problem with one of the plugins that are installed in that Docker image - the Could not load apps in space part of that error message isn't coming from the core CLI.

fwilhe commented 5 years ago

Hi Tom,

thanks for the reply.

“Subaccount" is a SAP Cloud Platform term which translates to “Organisation” in CF.

The message Could not load apps in space seems to be from the blue green deploy plugin which we do use.

Where it actually fails is cf login from this call

I could reproduce the error with this command, where my_user is not part of CloudApps or my_space, but both do exist.

$ cf login -u my_user -a my_endpoint -o CloudApps -s my_space
API endpoint: my_endpoint

Password>
Authenticating...
OK

API endpoint:   my_endpoint (API version: 2.125.0)
User:           my_user
No org or space targeted, use 'cf target -o ORG -s SPACE'
FAILED
Error finding org CloudApps
Organization CloudApps not found

I think that No org or space targeted is a bit misleading in this context, because they are targeted, it is "just" a permission issue. From a usability point of view, it would be much better not to create this false impression.

What I would suggest is to change the message No org or space targeted to indicate that it might also be a permission issue, like No org or space targeted, or missing permissions.

abbyachau commented 5 years ago

Please team could you look at whether we can easily/feasibly return a more helpful error in this particular use case, or is is it too difficult to identify this particular failure and return this particular error message. Thank you.

tjvman commented 5 years ago

So, the org isn't targeted in this case - we don't save off its identifying info if the current user isn't able to access it. That No org or space targeted (and the API endpoint and User info right above it) is an accurate representation of the current state.

The actual error here is Error finding org Cloud Apps Organization CloudApps not found - which, I agree, does not indicate that there was a permissions issue. We could modify the error message to also mention that the user may have had insufficient permissions - something like Organization CloudApps not found, or missing permissions, for example. Would that make the output more clear?

fwilhe commented 5 years ago

Would that make the output more clear?

Hey @tjvman, yes, I think that is better. Thank you for taking the time for this issue.

abbyachau commented 5 years ago

Hi @fwilhe thanks again for reaching out. We are currently rewriting the cf login command. After we complete the rewrite, we would be happy to take contributions for this update. Thanks again.