-
Notifications
You must be signed in to change notification settings - Fork 230
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 canI checks for route custom hosts #1935
Conversation
This adds @smarterclayton confirming all certificate fields (certificate, key, CA cert, destination CA cert) should be readonly when the |
[test] |
Disable hostname and certificate inputs if the user doesn't have `canI` check returns false for `routes/custom-host`.
2800ed1
to
6cc4254
Compare
Opened #1937 for the test flake [test] |
Evaluated for origin web console test up to 6cc4254 |
Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_web_console/43/) (Base Commit: 56c925a) (PR Branch Commit: 6cc4254) |
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
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.
Still digesting the original issue details, but changes look good to me!
[merge][severity:bug] |
Evaluated for origin web console merge up to 6cc4254 |
Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin_web_console/73/) (Base Commit: 6cd2686) (PR Branch Commit: 6cc4254) (Extended Tests: bug) |
May I ask why it's not OK to update certs? Having a bit of trouble with this in the dev preview where i can't get letsencrypt working as the controller needs to patch the routes with the renewed certs |
See openshift/origin#15772
@smarterclayton FYI