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
Bug 1802580: return ProjectRequestMessage if it is set with login,project cmds #406
Bug 1802580: return ProjectRequestMessage if it is set with login,project cmds #406
Conversation
@sallyom: This pull request references Bugzilla bug 1802577, which is invalid:
Comment In response to this:
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. |
to test $ oc edit project.config.openshift.io/cluster add this: spec:
projectRequestMessage: Contact your system administrator admin@company.com $ oc adm policy remove-cluster-role-from-group self-provisioner system:authenticated:oauth
$ oc login -u user1 -p pass1
Login successful.
Contact your system administrator admin@company.com
$ oc project
Contact your system administrator admin@company.com
$ oc new-project testproj
Error from server (Forbidden): Contact your system administrator admin@company.com |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
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. |
da87a64
to
8e0c245
Compare
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
1 similar comment
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
6 similar comments
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
d086566
to
20e2137
Compare
/retest |
1 similar comment
/retest |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
1 similar comment
@sallyom: This pull request references Bugzilla bug 1802580, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two nits and you're good to go.
pkg/cli/project/project.go
Outdated
} | ||
if err := client.RESTClient().Get().Resource("projectrequests").Do(context.TODO()).Into(&metav1.Status{}); err != nil { | ||
fmt.Fprintf(o.Out, "%v\n", err) | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is error, just return err
is the best option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated, fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but this changes behavior of oc project
to return an error.
As/is, if user w/out self-provisioner role runs oc project
they'll see this:
$ oc project
No project has been set. Pass a project name to make that the default.
and now, with this PR, it will be this:
$ oc project
Error from server (Forbidden): This is a friendly message...
or, if no projectRequestMessageSet, this:
$ oc project
Error from server (Forbidden): You may not request a new project via this API.
(this message without the set projRequestMessage is now consistent with the error a user gets with oc new-project foo
now, which makes more sense to me)
…lf-provisioner clusterrole
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sallyom, soltysh 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
@sallyom: All pull requests linked via external trackers have merged: openshift/oc#406. Bugzilla bug 1802580 has been moved to the MODIFIED state. In response to this:
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. |
This PR updates return msg to users that cannot create projects for
oc login
,oc new-project
, andoc project
If ProjectRequestMessage is set, and user without self-provisioner clusterrole logs in or tries to create projects, return the ProjectRequestMessage. This is consistent with console.
If no ProjectRequestMessage set, then return the same message for login|project|new-project:
oc project
without this PR is misleading:see comment below to set projectRequestMsg and remove self-provisioner clusterrole