-
Notifications
You must be signed in to change notification settings - Fork 1.9k
OSDOCS#13454: Updates to the web console tutorial #91212
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
OSDOCS#13454: Updates to the web console tutorial #91212
Conversation
|
🤖 Thu Apr 10 12:51:44 - Prow CI generated the docs preview: https://91212--ocpdocs-pr.netlify.app/ |
f32a9f9 to
0d1f376
Compare
cbf51bf to
51d6d50
Compare
51d6d50 to
37c7681
Compare
cca5f2b to
190df96
Compare
| * You are logged in to the {product-title} web console. | ||
| * You are in the *Developer* perspective. | ||
| * You have the appropriate roles and permissions in a project to create applications and other workloads in {product-title}. | ||
| // TODO: This requirement isn't in the CLI version. Is it necessary? |
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.
@AlexonOliveiraRH I think we can remove the above line ("You have the appropriate roles..."). It's not in the CLI version. wdyt?
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.
@AlexonOliveiraRH I think we can remove the above line ("You have the appropriate roles..."). It's not in the CLI version. wdyt?
@bergerhoffer although this is not CLI, permissions are bound across all clusters resources, independently of which platform you're using, i.e. CLI or web console, the user still needs to guarantee the proper permissions.
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.
@AlexonOliveiraRH What permissions should we say that they need to have then? (And if we add it here, we should also add that to the CLI version)
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.
@AlexonOliveiraRH What permissions should we say that they need to have then? (And if we add it here, we should also add that to the CLI version)
@bergerhoffer the minimum permission a user should have within a namespace scope to be able to deploy applications like this is the edit one, or the cluster admin could give the user also the admin permission within the namespace context only, which is a broader permission. More on this here: https://91212--ocpdocs-pr.netlify.app/openshift-enterprise/latest/authentication/using-rbac#default-roles_using-rbac
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.
Hm, this still doesn't seem to check out to me, because if they are the ones that created the project, then shouldn't they automatically get whatever permissions needed to work in that project?
The only thing I could see saying here is about whether someone has access to the cluster, but doesn't have permission to create a project (if there is such a role like that). wdyt?
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.
Hm, this still doesn't seem to check out to me, because if they are the ones that created the project, then shouldn't they automatically get whatever permissions needed to work in that project?
The only thing I could see saying here is about whether someone has access to the cluster, but doesn't have permission to create a project (if there is such a role like that). wdyt?
@bergerhoffer for example, the same user could get only a view permission, so this user would be able to navigate into the project, see things, but wouldn't be able to create nothing, neither change anything. That's why the cluster admin should grant this user at least edit permission (or admin in the namespace scope only) to create the project and do the rest of the tutorial. Also, the cluster admin could tweak RoleBindings to allow an user only to create a project, but not creating anything else inside it, but this is not a default behavior/permission.
|
@bergerhoffer well done! Everything worked like a charm. With the exception of some minor tweaking you may do, LGTM to move forward. |
190df96 to
71e0935
Compare
|
/label peer-review-needed |
|
Note to peer reviewer: FYI that a lot of the wording updates (like procedure intros, etc.) match what was already merged for the CLI version of the tutorial in #90215. |
kcarmichael08
left a comment
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.
This looks great! Had a few suggestions/questions but nothing big.
71e0935 to
edd229d
Compare
|
@AlexonOliveiraRH This is ready for another review. If it helps, here's the diff of what I changed since the last time you reviewed: https://github.com/openshift/openshift-docs/compare/71e0935bd6792e1fca1d06db0995474cebe4d99e..edd229dc788dea603795722c4fc2b4a6f7b3c7ed The main updates I'd want you to check are
|
@bergerhoffer these changes LGTM, thank you. The only it needs to be fixed is exactly the permission part. Please, if you could change from |
c4f5c82 to
1d43fbc
Compare
|
@AlexonOliveiraRH Okay here are my latest updates after our last discussions in Slack. I also went through and tested using Developer Sandbox, and found a few more little tweaks to make.
Here's the diff if it helps: https://github.com/openshift/openshift-docs/compare/edd229dc788dea603795722c4fc2b4a6f7b3c7ed..1d43fbcf05161eeb56fbbf0c0c2377e6a9f89b6d Let me know how these updates look, thanks! |
1d43fbc to
9584c07
Compare
Nice.
Good catch.
Good catch too.
Sounds good to me.
Perfect.
Also a good catch.
Nice touch.
@bergerhoffer I just finished my review and kudos to you, ma'am. It looks very good to me. :) I think you addressed all the main points we discussed, specially the recent ones. |
|
@yapei Here is the PR to update the web console tutorial. Can you PTAL when you get a chance? Preview: https://91212--ocpdocs-pr.netlify.app/openshift-enterprise/latest/tutorials/dev-app-web-console Similar changes were made to the CLI version of the tutorial in #90215, which has been approved/merged. This PR also makes a few further tweaks to the CLI version that we made after working on this web console version. Let me know if you have any questions, thanks! |
|
@bergerhoffer Changes look perfect from QE's point of view, some tiny update requests above, PTAL |
9584c07 to
c3874ae
Compare
|
@bergerhoffer: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
|
@yapei I've updated the two items you pointed out, if you can take another look please. Thanks! |
|
Thanks @bergerhoffer LGTM |
|
/cherrypick enterprise-4.19 |
|
@bergerhoffer: new pull request created: #92099 DetailsIn 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-sigs/prow repository. |
Version(s):
4.19
Issue:
https://issues.redhat.com/browse/OSDOCS-13254
Link to docs preview:
https://91212--ocpdocs-pr.netlify.app/openshift-enterprise/latest/tutorials/dev-app-web-console
QE review:
Additional information:
Will need #90215 to merge first before can make changes to the web console assembly.DoneNote to reviewer: Some of the wording updates match what was already merged for the CLI version of the tutorial in #90215.