Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CFE-987: Use RouterExternalCertificate feature gate for adding ROUTER_ENABLE_EXTERNAL_CERTIFICATE env var to the router pods #1017

Merged
merged 1 commit into from
Feb 27, 2024
Merged

CFE-987: Use RouterExternalCertificate feature gate for adding ROUTER_ENABLE_EXTERNAL_CERTIFICATE env var to the router pods #1017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions pkg/operator/controller/ingress/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,9 @@ func enqueueRequestForOwningIngressController(namespace string) handler.EventHan

// Config holds all the things necessary for the controller to run.
type Config struct {
Namespace string
IngressControllerImage string
Namespace string
IngressControllerImage string
RouteExternalCertificateEnabled bool
}

// reconciler handles the actual ingress reconciliation logic in response to
Expand Down
11 changes: 9 additions & 2 deletions pkg/operator/controller/ingress/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (r *reconciler) ensureRouterDeployment(ci *operatorv1.IngressController, in
if err != nil {
return false, nil, fmt.Errorf("failed to determine if proxy protocol is needed for ingresscontroller %s/%s: %v", ci.Namespace, ci.Name, err)
}
desired, err := desiredRouterDeployment(ci, r.config.IngressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, haveClientCAConfigmap, clientCAConfigmap, clusterProxyConfig)
desired, err := desiredRouterDeployment(ci, r.config.IngressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, haveClientCAConfigmap, clientCAConfigmap, clusterProxyConfig, r.config.RouteExternalCertificateEnabled)
if err != nil {
return haveDepl, current, fmt.Errorf("failed to build router deployment: %v", err)
}
Expand Down Expand Up @@ -241,7 +241,7 @@ func headerValues(values []operatorv1.IngressControllerHTTPHeader) string {
}

// desiredRouterDeployment returns the desired router deployment.
func desiredRouterDeployment(ci *operatorv1.IngressController, ingressControllerImage string, ingressConfig *configv1.Ingress, infraConfig *configv1.Infrastructure, apiConfig *configv1.APIServer, networkConfig *configv1.Network, proxyNeeded bool, haveClientCAConfigmap bool, clientCAConfigmap *corev1.ConfigMap, clusterProxyConfig *configv1.Proxy) (*appsv1.Deployment, error) {
func desiredRouterDeployment(ci *operatorv1.IngressController, ingressControllerImage string, ingressConfig *configv1.Ingress, infraConfig *configv1.Infrastructure, apiConfig *configv1.APIServer, networkConfig *configv1.Network, proxyNeeded bool, haveClientCAConfigmap bool, clientCAConfigmap *corev1.ConfigMap, clusterProxyConfig *configv1.Proxy, routeExternalCertificateEnabled bool) (*appsv1.Deployment, error) {
deployment := manifests.RouterDeployment()
name := controller.RouterDeploymentName(ci)
deployment.Name = name.Name
Expand Down Expand Up @@ -1123,6 +1123,13 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
)
}

// Add router external certificate environment variable.
if routeExternalCertificateEnabled {
env = append(env,
corev1.EnvVar{Name: "ROUTER_ENABLE_EXTERNAL_CERTIFICATE", Value: "true"},
)
}

// TODO: The only connections from the router that may need the cluster-wide proxy are those for downloading CRLs,
// which, as of writing this, will always be http. If https becomes necessary, the router will need to mount the
// trusted CA bundle that cluster-network-operator generates. The process for adding that is described here:
Expand Down
56 changes: 45 additions & 11 deletions pkg/operator/controller/ingress/deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func TestTuningOptions(t *testing.T) {
ic.Spec.TuningOptions.HealthCheckInterval = &metav1.Duration{Duration: 15 * time.Second}
ic.Spec.TuningOptions.ReloadInterval = metav1.Duration{Duration: 30 * time.Second}

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -198,7 +198,7 @@ func TestTuningOptions(t *testing.T) {
func TestClusterProxy(t *testing.T) {
ic, ingressConfig, infraConfig, apiConfig, networkConfig, _, clusterProxyConfig := getRouterDeploymentComponents(t)

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand All @@ -222,7 +222,7 @@ func TestClusterProxy(t *testing.T) {
clusterProxyConfig.Status.HTTPSProxy = "bar"
clusterProxyConfig.Status.NoProxy = "baz"

deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -387,7 +387,7 @@ func getRouterDeploymentComponents(t *testing.T) (*operatorv1.IngressController,
func Test_desiredRouterDeployment(t *testing.T) {
ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -468,7 +468,7 @@ func Test_desiredRouterDeployment(t *testing.T) {
func TestDesiredRouterDeploymentSpecTemplate(t *testing.T) {
ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -597,7 +597,7 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
if err != nil {
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
}
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -684,7 +684,7 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
if err != nil {
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
}
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -799,7 +799,7 @@ func TestDesiredRouterDeploymentVariety(t *testing.T) {
if err != nil {
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
}
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -913,7 +913,7 @@ func TestDesiredRouterDeploymentHostNetworkNil(t *testing.T) {
if err != nil {
t.Fatal(err)
}
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -944,7 +944,7 @@ func TestDesiredRouterDeploymentSingleReplica(t *testing.T) {
},
}

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -2248,7 +2248,7 @@ func TestDesiredRouterDeploymentDefaultPlacement(t *testing.T) {
// This value does not matter in the context of this test, just use a dummy value
dummyProxyNeeded := true

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, tc.ingressConfig, tc.infraConfig, apiConfig, networkConfig, dummyProxyNeeded, false, nil, &configv1.Proxy{})
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, tc.ingressConfig, tc.infraConfig, apiConfig, networkConfig, dummyProxyNeeded, false, nil, &configv1.Proxy{}, false)
if err != nil {
t.Error(err)
}
Expand All @@ -2265,3 +2265,37 @@ func TestDesiredRouterDeploymentDefaultPlacement(t *testing.T) {
}

}

func TestDesiredRouterDeploymentRouterExternalCertificate(t *testing.T) {
ic, ingressConfig, infraConfig, apiConfig, networkConfig, _, clusterProxyConfig := getRouterDeploymentComponents(t)

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}

// Verify that router external certificate env var is not added.
expectedEnv := []envData{
{"ROUTER_ENABLE_EXTERNAL_CERTIFICATE", false, ""},
}

if err := checkDeploymentEnvironment(t, deployment, expectedEnv); err != nil {
t.Error(err)
}

deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, true)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}

// Verify that router external certificate env var is set to true.
expectedEnv = []envData{
{"ROUTER_ENABLE_EXTERNAL_CERTIFICATE", true, "true"},
}

if err := checkDeploymentEnvironment(t, deployment, expectedEnv); err != nil {
t.Error(err)
}

checkDeploymentHasEnvSorted(t, deployment)
}
6 changes: 4 additions & 2 deletions pkg/operator/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
azureWorkloadIdentityEnabled := featureGates.Enabled(configv1.FeatureGateAzureWorkloadIdentity)
sharedVPCEnabled := featureGates.Enabled(configv1.FeatureGatePrivateHostedZoneAWS)
gatewayAPIEnabled := featureGates.Enabled(configv1.FeatureGateGatewayAPI)
routeExternalCertificateEnabled := featureGates.Enabled(configv1.FeatureGateRouteExternalCertificate)

// Set up an operator manager for the operator namespace.
mgr, err := manager.New(kubeConfig, manager.Options{
Expand Down Expand Up @@ -163,8 +164,9 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro

// Create and register the ingress controller with the operator manager.
if _, err := ingresscontroller.New(mgr, ingresscontroller.Config{
Namespace: config.Namespace,
IngressControllerImage: config.IngressControllerImage,
Namespace: config.Namespace,
IngressControllerImage: config.IngressControllerImage,
RouteExternalCertificateEnabled: routeExternalCertificateEnabled,
}); err != nil {
return nil, fmt.Errorf("failed to create ingress controller: %v", err)
}
Expand Down