Add Default Apps Domain enhancement #432
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
/assign @knobunc |
dustman9000
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
/label tide/merge-method-squash |
openshift-ci-robot
added
the
tide/merge-method-squash
dustman9000
force-pushed
the
add-apps-default-domain
branch
from
f702d25
to
c751681
Compare
|
/assign @knobunc |
|
/assign @danehans |
Yes, the NE RFE is https://issues.redhat.com/browse/RFE-952 |
|
|
||
| Certain routes are created by various operators at install time. These routes | ||
| could be potentially be deleted and re-added post-install time. Implementation | ||
| must ensure these routes use the original domain when re-added. |
There was a problem hiding this comment.
Does "Implementation" means whatever operator creates the route? For example, cluster-foo-operator creates route "foo", and after this enhancement, cluster-foo-operator must ensure that it specifies a value for spec.host based on domain from the cluster ingress config when the operator creates route "foo".
There was a problem hiding this comment.
That's correct. The concern here is that if the route was created by an operator without a spec.host value, if the route is re-added it could take on the new spec.appsDomain value. Let me know if I should keep this out of the "implementation" section. I wasn't sure where to raise this concern.
There was a problem hiding this comment.
Console and auth operators come to mind...after this enhancement is implemented, should the console/auth operators be specifying spec.host on their owned routes for 4.7 going forward? And should those operators should also assume that the original cluster domain is the intended host? In other words, there isn't a need for the console and auth routes to be moved under the new app default domain, right?
There was a problem hiding this comment.
@sgreene570 - Correct, there is no need to move the console/oauth routes under the new domain. For now, since this enhancement is primarily for user-created routes, I would suggest we restrict this enhancement to user-created namespaces (ie. not in openshift-* and redhat-*).
There was a problem hiding this comment.
@sgreene570, note that setting appsDomain is day-2 (post-install) configuration. This means that operators will have already created the routes, and the API will have set default values for spec.host based on the ingress domain if the operators did not specify spec.host. The only concern is that the cluster administrator could modify appsDomain and something could subsequently delete the operator-created routes, and if the operators then recreated the routes without specifying an explicit spec.host value, they API would use the wrong domain (appsDomain instead of domain) for the operator-created routes.
There was a problem hiding this comment.
The authentication operator specifies spec.host: https://github.com/openshift/cluster-authentication-operator/blob/b9d7a84f9db82d0e622b0d0b9c5d9162e810985f/pkg/controllers/metadata/metadata_controller.go#L245
The console operator does not: https://github.com/openshift/console-operator/blob/b195cebf52ea7af30178f4704ac269f482d70208/pkg/console/subresource/route/route.go#L69-L85
@spadgett, to address this contingency, it might make sense for the console operator to do something similar to the authentication operator for the console's default route.
dustman9000
force-pushed
the
add-apps-default-domain
branch
from
05417c1
to
f8c474e
Compare
…pps-default-domain
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dustman9000, knobunc, Miciah 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 |
openshift-ci-robot
added
the
approved
This adds the Default Apps Domain enhancement.
Associated NE RFE: https://issues.redhat.com/browse/RFE-952