Skip to content

Conversation

@eparis
Copy link
Member

@eparis eparis commented Apr 11, 2019

wired to the same load balancer. But does mean you can change the certs
and CA for the apiserver on the public name, but let us continue to own
certs for the -int name used by kubelets

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 11, 2019
eparis added 2 commits April 11, 2019 15:45
wired to the same load balancer. But does mean you can change the certs
and CA for the apiserver on the public name, but let us continue to own
certs for the -int name.
@abhinavdahiya abhinavdahiya changed the title data/aws: create an api-int dns name BUG 1685704: data/aws: create an api-int dns name Apr 11, 2019
@abhinavdahiya
Copy link
Contributor

/lgtm
/retest

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, eparis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 11, 2019
@openshift-merge-robot openshift-merge-robot merged commit e3eb2e2 into openshift:master Apr 12, 2019
wking added a commit to wking/openshift-installer that referenced this pull request Apr 30, 2019
…-int

Catching up with 13e4b70 (data/aws: create an api-int dns name,
2019-04-11, openshift#1601) and 052fcee (asset/manifests: use internal
apiserver name, 2019-04-17, openshift#1633).
wking added a commit to wking/openshift-installer that referenced this pull request May 2, 2019
Catching up with 13e4b70 (data/aws: create an api-int dns name,
2019-04-11, openshift#1601), now that 052fcee (asset/manifests: use internal
apiserver name, 2019-04-17, openshift#1633) has moved some internal assets over
to that name.
wking added a commit to wking/openshift-api that referenced this pull request May 6, 2019
…ernal

The kubelet (via the machine-config operator [1]) and Kubernetes
API-server operator [2] need the internal API URI.  The console
operator [3] and authentication operator [4] need the external URI.
Since the api-int split (e.g. openshift/installer@13e4b702f7,
data/aws: create an api-int dns name, 2019-04-11,
openshift/installer#1601), those are two different things, so we need
two different properties.

I've deprecated the old property because it was ambiguous.  We can
drop it if/when we ever cut a v2 of this type.

I've avoided leading with External or Internal so these properties
will all alphabetize together when rendering YAML, etc.

I've avoided "Public", because that is often used in a public/private
dichotomy for security.  The split was motivated by separating X.509
chains of trust, but the URIs themselves are not sensitive.

I've also taken the opportunity to change from URL to the more-generic
URI, because we don't need to make a point of locator-ness and RFCs
recommend using the more generic label [5,6].

[1]: https://github.com/openshift/machine-config-operator/blob/2bd29e5005fb6f438f1b1c512176c3f7e5cf9e47/pkg/operator/operator.go#L393
[2]: https://github.com/openshift/cluster-kube-apiserver-operator/blob/cbcb3f403f674dd8d0b590d770c0ebf6ecb75f17/pkg/operator/certrotationcontroller/externalloadbalancer.go#L18
[3]: https://github.com/openshift/console-operator/blob/fc721a6990b6ca27efb60062cf5e9d620ec0ab01/pkg/console/subresource/configmap/configmap.go#L59
[4]: https://github.com/openshift/cluster-authentication-operator/blob/96643e4b29452f9ad20f7b90fc0794ffc3006fa2/pkg/operator2/oauth.go#L117
[5]: https://tools.ietf.org/html/rfc3305#section-2.2
[6]: https://tools.ietf.org/html/rfc3986#section-1.1.3
wking added a commit to wking/openshift-api that referenced this pull request May 6, 2019
…rverURL

The kubelet (via the machine-config operator [1]) and Kubernetes
API-server operator [2] need the internal API URI.  The console
operator [3] and authentication operator [4] need the external URI.
Since the api-int split (e.g. openshift/installer@13e4b702f7,
data/aws: create an api-int dns name, 2019-04-11,
openshift/installer#1601), those are two different things, so we need
two different properties.

I'd originally proposed deprecating the old property because it was
ambiguous, but Clayton suggested using the generic name for the
external URI because the internal URI is only intended for a small
subset of components [5].

I've avoided leading with Internal so these properties will all
alphabetize together when rendering YAML, etc.

I've avoided "Private", because that is often used in a public/private
dichotomy for security.  The split was motivated by separating X.509
chains of trust, but the URIs themselves are not sensitive.

I'd originally proposed changing URL to URI, because we don't need to
make a point of locator-ness and RFCs recommend using the more generic
label [5,6].  But Sam and Clayton prefer sticking with URL, because it
is used more widely within OpenShift [7,8].

[1]: https://github.com/openshift/machine-config-operator/blob/2bd29e5005fb6f438f1b1c512176c3f7e5cf9e47/pkg/operator/operator.go#L393
[2]: https://github.com/openshift/cluster-kube-apiserver-operator/blob/cbcb3f403f674dd8d0b590d770c0ebf6ecb75f17/pkg/operator/certrotationcontroller/externalloadbalancer.go#L18
[3]: https://github.com/openshift/console-operator/blob/fc721a6990b6ca27efb60062cf5e9d620ec0ab01/pkg/console/subresource/configmap/configmap.go#L59
[4]: https://github.com/openshift/cluster-authentication-operator/blob/96643e4b29452f9ad20f7b90fc0794ffc3006fa2/pkg/operator2/oauth.go#L117
[5]: openshift#308 (comment)
[5]: https://tools.ietf.org/html/rfc3305#section-2.2
[6]: https://tools.ietf.org/html/rfc3986#section-1.1.3
[7]: openshift#308 (comment)
[8]: openshift#308 (comment)
wking added a commit to wking/openshift-api that referenced this pull request May 6, 2019
…rverURL

The kubelet (via the machine-config operator [1]) and Kubernetes
API-server operator [2] need the internal API URI.  The console
operator [3] and authentication operator [4] need the external URI.
Since the api-int split (e.g. openshift/installer@13e4b702f7,
data/aws: create an api-int dns name, 2019-04-11,
openshift/installer#1601), those are two different things, so we need
two different properties.

I'd originally proposed deprecating the old property because it was
ambiguous, but Clayton suggested using the generic name for the
external URI because the internal URI is only intended for a small
subset of components [5].

I've avoided leading with Internal so these properties will all
alphabetize together when rendering YAML, etc.

I've avoided "Private", because that is often used in a public/private
dichotomy for security.  The split was motivated by separating X.509
chains of trust, but the URIs themselves are not sensitive.

I'd originally proposed changing URL to URI, because we don't need to
make a point of locator-ness and RFCs recommend using the more generic
label [5,6].  But Sam and Clayton prefer sticking with URL, because it
is used more widely within OpenShift [7,8].

[1]: https://github.com/openshift/machine-config-operator/blob/2bd29e5005fb6f438f1b1c512176c3f7e5cf9e47/pkg/operator/operator.go#L393
[2]: https://github.com/openshift/cluster-kube-apiserver-operator/blob/cbcb3f403f674dd8d0b590d770c0ebf6ecb75f17/pkg/operator/certrotationcontroller/externalloadbalancer.go#L18
[3]: https://github.com/openshift/console-operator/blob/fc721a6990b6ca27efb60062cf5e9d620ec0ab01/pkg/console/subresource/configmap/configmap.go#L59
[4]: https://github.com/openshift/cluster-authentication-operator/blob/96643e4b29452f9ad20f7b90fc0794ffc3006fa2/pkg/operator2/oauth.go#L117
[5]: openshift#308 (comment)
[5]: https://tools.ietf.org/html/rfc3305#section-2.2
[6]: https://tools.ietf.org/html/rfc3986#section-1.1.3
[7]: openshift#308 (comment)
[8]: openshift#308 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants