-
Notifications
You must be signed in to change notification settings - Fork 59
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
KIE-ISSUES 1056 - Introduce AuthZ Guide for SonataFlow Deployments #614
Conversation
Signed-off-by: Ricardo Zanini <[email protected]>
🎊 PR Preview bdb1690 has been successfully built and deployed. See the documentation preview: https://sonataflow-docs-preview-pr-614.surge.sh |
Signed-off-by: Ricardo Zanini <[email protected]>
Signed-off-by: Ricardo Zanini <[email protected]>
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.
@ricardozanini I did first round of review (mostly little typos).
I noticed 2 more points:
- Sometimes you are using modal auxiliary verbs as "will", "should", "can". Shouldn't we use just present tense? + removing you're
- Looking at the tooling guides and we use bold labels for the UI components. In this guide you use apostrophes. I would suggest to use also the bold text. Wdyt?
If you agree, I can go through the guide and make all the suggestions. 🙂
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
Nice suggestions, please go ahead! |
Co-authored-by: Tomáš David <[email protected]>
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.
I did a second round, I tried to remove the future tense and replace ui componentss apostrophes with the bold text.
I will do one more to test the documentation together with the example.
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
Co-authored-by: Tomáš David <[email protected]>
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.
@ricardozanini Thanks a lot for all the changes 😊. Few more findings when I tested the example together with the documentation.
- I missed few more ui elements and names to be wrapped with ` or *.
- I updated
02-sonataflow-route.yaml
to be exactly same as in the example. - I replaced
INGRESS_PORT
withINGRESS_URL
. The url may differs on different environmetns.
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
Co-authored-by: Tomáš David <[email protected]>
Thank you @tomasdavidorg I appreciate the thorough review! You can check now your changes in the preview. |
Checked and looks good to me. Thanks again 🙂. |
Thank you for checking, this one was a hell to put together. |
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
…hz.adoc Co-authored-by: Walter Medvedeo <[email protected]>
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, I have added a couple of nitpicks if you want to consider adding.
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
Co-authored-by: Walter Medvedeo <[email protected]>
@kaldesai can you ptal? |
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
serverlessworkflow/modules/ROOT/pages/cloud/custom-ingress-authz.adoc
Outdated
Show resolved
Hide resolved
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.
Minor suggestions! Otherwise, your content looks good to me. Very well written. Thank you!
Co-authored-by: Kalyani Desai <[email protected]>
Closes apache/incubator-kie-issues#1056
Depends on apache/incubator-kie-kogito-examples#1902
Description:
Please make sure that your PR meets the following requirements:
Issue-XYZ Subject
[0.9.x] Issue-XYZ Subject
How to backport a pull request to a different branch?
In order to automatically create a backporting pull request please add one or more labels having the following format
backport-<branch-name>
, where<branch-name>
is the name of the branch where the pull request must be backported to (e.g.,backport-7.67.x
to backport the original PR to the7.67.x
branch).Once the original pull request is successfully merged, the automated action will create one backporting pull request per each label (with the previous format) that has been added.
If something goes wrong, the author will be notified and at this point a manual backporting is needed.