Add TLS adherence feature gate #2680
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| apiVersion: apiextensions.k8s.io/v1 | ||
| name: "APIServer" | ||
| crdName: apiservers.config.openshift.io | ||
| featureGates: | ||
| - TLSAdherence | ||
| tests: | ||
| onCreate: | ||
| - name: Should be able to create without tlsAdherence | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: {} | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| audit: | ||
| profile: Default | ||
| - name: Should be able to create with tlsAdherence set to LegacyAdheringComponentsOnly | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: LegacyAdheringComponentsOnly | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| audit: | ||
| profile: Default | ||
| tlsAdherence: LegacyAdheringComponentsOnly | ||
| - name: Should be able to create with tlsAdherence set to StrictAllComponents | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: StrictAllComponents | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| audit: | ||
| profile: Default | ||
| tlsAdherence: StrictAllComponents | ||
| - name: Should reject invalid tlsAdherence value | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: InvalidValue | ||
| expectedError: 'spec.tlsAdherence: Unsupported value: "InvalidValue": supported values: "LegacyAdheringComponentsOnly", "StrictAllComponents"' | ||
| onUpdate: | ||
| - name: Should be able to update tlsAdherence from LegacyAdheringComponentsOnly to StrictAllComponents | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: LegacyAdheringComponentsOnly | ||
| updated: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: StrictAllComponents | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| audit: | ||
| profile: Default | ||
| tlsAdherence: StrictAllComponents | ||
| - name: Should be able to update tlsAdherence from StrictAllComponents to LegacyAdheringComponentsOnly | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: StrictAllComponents | ||
| updated: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: LegacyAdheringComponentsOnly | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| audit: | ||
| profile: Default | ||
| tlsAdherence: LegacyAdheringComponentsOnly | ||
| - name: Should be able to set tlsAdherence from unset on update | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: {} | ||
| updated: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: StrictAllComponents | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| audit: | ||
| profile: Default | ||
| tlsAdherence: StrictAllComponents | ||
| - name: Should not allow removing tlsAdherence once set | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: | ||
| tlsAdherence: StrictAllComponents | ||
| updated: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: APIServer | ||
| spec: {} | ||
| expectedError: "tlsAdherence may not be removed once set" | ||
richardsonnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ type APIServer struct { | |
| Status APIServerStatus `json:"status"` | ||
| } | ||
|
|
||
| // +openshift:validation:FeatureGateAwareXValidation:featureGate=TLSAdherence,rule="has(oldSelf.tlsAdherence) ? has(self.tlsAdherence) : true",message="tlsAdherence may not be removed once set" | ||
| type APIServerSpec struct { | ||
| // servingCert is the TLS cert info for serving secure traffic. If not specified, operator managed certificates | ||
| // will be used for serving secure traffic. | ||
|
|
@@ -62,6 +63,39 @@ type APIServerSpec struct { | |
| // The current default is the Intermediate profile. | ||
| // +optional | ||
| TLSSecurityProfile *TLSSecurityProfile `json:"tlsSecurityProfile,omitempty"` | ||
| // tlsAdherence controls if components in the cluster adhere to the TLS security profile | ||
| // configured on this APIServer resource. | ||
| // | ||
| // Valid values are "LegacyAdheringComponentsOnly" and "StrictAllComponents". | ||
| // | ||
| // When set to "LegacyAdheringComponentsOnly", components that already honor the | ||
| // cluster-wide TLS profile continue to do so. Components that do not already honor | ||
| // it continue to use their individual TLS configurations. | ||
| // | ||
| // When set to "StrictAllComponents", all components must honor the configured TLS | ||
| // profile unless they have a component-specific TLS configuration that overrides | ||
| // it. This mode is recommended for security-conscious deployments and is required | ||
| // for certain compliance frameworks. | ||
| // | ||
| // Note: Some components such as Kubelet and IngressController have their own | ||
| // dedicated TLS configuration mechanisms via KubeletConfig and IngressController | ||
| // CRs respectively. When these component-specific TLS configurations are set, | ||
| // they take precedence over the cluster-wide tlsSecurityProfile. When not set, | ||
| // these components fall back to the cluster-wide default. | ||
| // | ||
| // Components that encounter an unknown value for tlsAdherence should treat it | ||
| // as "StrictAllComponents" and log a warning to ensure forward compatibility | ||
| // while defaulting to the more secure behavior. | ||
| // | ||
| // This field is optional. | ||
| // When omitted, this means the user has no opinion and the platform is left | ||
| // to choose reasonable defaults. These defaults are subject to change over time. | ||
| // The current default is LegacyAdheringComponentsOnly. | ||
| // | ||
| // Once set, this field may be changed to a different value, but may not be removed. | ||
| // +openshift:enable:FeatureGate=TLSAdherence | ||
| // +optional | ||
|
Member
There was a problem hiding this comment. We need to configure validation on this field, and set the default.
Member
There was a problem hiding this comment. Oh I see you are defining the Enum validation on the type, which is fine. Let's also include the
Member
There was a problem hiding this comment. Hmm, reading the tlsAdherence description again, it seems like you are wanting it to be possible for an empty string to be allowed to be persisted for this field in etcd, which would allow us in theory to change the default semantic later in the future from legacy to strict without a change in the actual tlsAdherence value. In practice, I think the problem with that idea is that each component essentially has their own implementation of this API and we'd have to do a really good job coordinating a semantic switch from legacy to strict. What do folks think about:
Contributor
There was a problem hiding this comment. For configuration APIs we generally try to avoid setting the default via In this case, I don't think we will be doing that but I still wouldn't use the
Member
There was a problem hiding this comment.
Thanks, I was hoping you'd have some good advice here. It is the intent to eventually make all components honor the cluster-wide default, so I supposed we'll just have to deal with trying to coordinate a semantic change across a bunch of components at once when the time comes. To facilitate that, I'd suggest a library-go function somehow like this: // in configv1
const TLSAdherenceNoOpinion = ""func ShouldAllComponentsAdhere(tlsAdherence configv1.TLSAdherence) bool {
if tlsAdherence == configv1.TLSAdherenceNoOpinion || tlsAdherence == configv1.LegacyExternalAPIServerComponentsOnly {
return false
}
return true
}And then when we want to change the semantic, we'd update that function to: func ShouldAllComponentsAdhere(tlsAdherence configv1.TLSAdherence) bool {
if tlsAdherence == configv1.LegacyExternalAPIServerComponentsOnly {
return false
}
return true
}
Contributor
There was a problem hiding this comment. That aligns with what I envisioned. Can we make sure to update the EP with a mention of this kind of check/library that implementors are expected to use?
Author
There was a problem hiding this comment. Added a mention of this helper function. Will need to land this first to get the TLSAdherencePolicy type into library-go There was a problem hiding this comment. The behavior for unknown values seem inconsistent with the behavior for unset/empty values. Shouldn't unknown values lead to a warning and setting the default value instead of
Contributor
There was a problem hiding this comment. I'm not super opinionated on what the "unknown" case means for components, but I think I suspect that we probably won't add many new allowed values here. Assuming that we do have a scenario where we've added a new value that might be unknown for some components, I think the current assumption of unknown != explicit legacy choice (that all components should be aware of) being that components should respect the configuration defined here is reasonable. Issuing some kind of warning sounds reasonable to me. You wouldn't want to modify this value based on one component not understanding the value here because other components may understand the value. |
||
| TLSAdherence TLSAdherencePolicy `json:"tlsAdherence,omitempty"` | ||
richardsonnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // audit specifies the settings for audit configuration to be applied to all OpenShift-provided | ||
| // API servers in the cluster. | ||
| // +optional | ||
|
|
@@ -237,6 +271,35 @@ const ( | |
| type APIServerStatus struct { | ||
| } | ||
|
|
||
| // TLSAdherencePolicy defines which components adhere to the TLS security profile. | ||
| // Implementors should use the ShouldHonorClusterTLSProfile helper function from library-go | ||
| // rather than checking these values directly. | ||
| // +kubebuilder:validation:Enum=LegacyAdheringComponentsOnly;StrictAllComponents | ||
| type TLSAdherencePolicy string | ||
richardsonnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const ( | ||
richardsonnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // TLSAdherencePolicyNoOpinion represents an empty/unset value for tlsAdherence. | ||
| // This value cannot be explicitly set and is only present when the field is omitted. | ||
| // When the field is omitted, the cluster defaults to LegacyAdheringComponentsOnly | ||
| // behavior. Components should treat this the same as LegacyAdheringComponentsOnly. | ||
| TLSAdherencePolicyNoOpinion TLSAdherencePolicy = "" | ||
|
|
||
| // TLSAdherencePolicyLegacyAdheringComponentsOnly maintains backward-compatible behavior. | ||
| // Components that already honor the cluster-wide TLS profile (such as kube-apiserver, | ||
| // openshift-apiserver, oauth-apiserver, and others) continue to do so. Components that do | ||
| // not already honor it continue to use their individual TLS configurations (e.g., | ||
| // IngressController.spec.tlsSecurityProfile, KubeletConfig.spec.tlsSecurityProfile, | ||
| // or component defaults). No additional components are required to start honoring the | ||
| // cluster-wide profile in this mode. | ||
| TLSAdherencePolicyLegacyAdheringComponentsOnly TLSAdherencePolicy = "LegacyAdheringComponentsOnly" | ||
|
|
||
| // TLSAdherencePolicyStrictAllComponents means all components must honor the configured TLS | ||
| // profile unless they have a component-specific TLS configuration that overrides it. | ||
| // This mode is recommended for security-conscious deployments and is required | ||
| // for certain compliance frameworks. | ||
| TLSAdherencePolicyStrictAllComponents TLSAdherencePolicy = "StrictAllComponents" | ||
| ) | ||
|
|
||
| // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
|
|
||
| // Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer). | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.