diff --git a/bindata/cloud-network-config-controller/managed/controller.yaml b/bindata/cloud-network-config-controller/managed/controller.yaml index 663653c204..b9af887581 100644 --- a/bindata/cloud-network-config-controller/managed/controller.yaml +++ b/bindata/cloud-network-config-controller/managed/controller.yaml @@ -165,6 +165,9 @@ spec: -platform-aws-ca-override={{.PlatformAWSCAPath}} \ -platform-azure-environment={{.PlatformAzureEnvironment}} \ -secret-name cloud-network-config-controller-creds \ +{{- if .OSMaxAllowedAddressPairs }} + -platform-os-max-allowed-address-pairs={{ .OSMaxAllowedAddressPairs }} \ +{{- end }} -kubeconfig /var/run/secrets/hosted_cluster/kubeconfig env: - name: CONTROLLER_NAMESPACE diff --git a/bindata/cloud-network-config-controller/self-hosted/controller.yaml b/bindata/cloud-network-config-controller/self-hosted/controller.yaml index 7a6b089066..3db62b6cd4 100644 --- a/bindata/cloud-network-config-controller/self-hosted/controller.yaml +++ b/bindata/cloud-network-config-controller/self-hosted/controller.yaml @@ -42,12 +42,18 @@ spec: image: {{.CloudNetworkConfigControllerImage}} imagePullPolicy: IfNotPresent command: ["/usr/bin/cloud-network-config-controller"] - args: [ "-platform-type", "{{.PlatformType}}", - "-platform-region={{.PlatformRegion}}", - "-platform-api-url={{.PlatformAPIURL}}", - "-platform-aws-ca-override={{.PlatformAWSCAPath}}", - "-platform-azure-environment={{.PlatformAzureEnvironment}}", - "-secret-name", "cloud-credentials"] + args: + - "-platform-type" + - "{{.PlatformType}}" + - "-platform-region={{.PlatformRegion}}" + - "-platform-api-url={{.PlatformAPIURL}}" + - "-platform-aws-ca-override={{.PlatformAWSCAPath}}" + - "-platform-azure-environment={{.PlatformAzureEnvironment}}" + - "-secret-name" + - "cloud-credentials" +{{- if .OSMaxAllowedAddressPairs }} + - "-platform-os-max-allowed-address-pairs={{ .OSMaxAllowedAddressPairs }}" +{{- end }} env: - name: CONTROLLER_NAMESPACE valueFrom: diff --git a/pkg/bootstrap/types.go b/pkg/bootstrap/types.go index d24b5fb4a4..3a0715066c 100644 --- a/pkg/bootstrap/types.go +++ b/pkg/bootstrap/types.go @@ -97,12 +97,20 @@ type IPTablesAlerterBootstrapResult struct { Enabled bool } +// CloudNetworkConfigBootstrapResult contains bootstrap configuration +// read from the cloud-network-config ConfigMap. +type CloudNetworkConfigBootstrapResult struct { + OSMaxAllowedAddressPairs *int +} + type BootstrapResult struct { Infra InfraStatus OVN OVNBootstrapResult IPTablesAlerter IPTablesAlerterBootstrapResult TLSProfile TLSProfile + + CloudNetworkConfig CloudNetworkConfigBootstrapResult } type InfraStatus struct { diff --git a/pkg/network/bootstrap.go b/pkg/network/bootstrap.go index b4575b7315..d826055d73 100644 --- a/pkg/network/bootstrap.go +++ b/pkg/network/bootstrap.go @@ -2,7 +2,10 @@ package network import ( "context" + "fmt" + "strconv" + configv1 "github.com/openshift/api/config/v1" operv1 "github.com/openshift/api/operator/v1" "github.com/openshift/cluster-network-operator/pkg/bootstrap" cnoclient "github.com/openshift/cluster-network-operator/pkg/client" @@ -34,6 +37,14 @@ func Bootstrap(conf *operv1.Network, client cnoclient.Client) (*bootstrap.Bootst out.IPTablesAlerter = iptablesAlerterBootstrap(client.ClientFor("").CRClient()) + if infraStatus.PlatformType == configv1.OpenStackPlatformType { + cnc, err := cloudNetworkConfigBootstrap(context.Background(), client.ClientFor("").CRClient()) + if err != nil { + return nil, err + } + out.CloudNetworkConfig = cnc + } + out.TLSProfile, err = GetTLSProfile(client, infraStatus.HostedControlPlane) if err != nil { return nil, err @@ -67,3 +78,31 @@ func iptablesAlerterBootstrap(cl crclient.Reader) bootstrap.IPTablesAlerterBoots return result } + +func cloudNetworkConfigBootstrap(ctx context.Context, cl crclient.Reader) (bootstrap.CloudNetworkConfigBootstrapResult, error) { + result := bootstrap.CloudNetworkConfigBootstrapResult{} + + cm := &corev1.ConfigMap{} + if err := cl.Get(ctx, types.NamespacedName{ + Namespace: "openshift-network-operator", + Name: "cloud-network-config", + }, cm); err != nil { + if !apierrors.IsNotFound(err) { + return result, fmt.Errorf("error fetching cloud-network-config configmap: %w", err) + } + return result, nil + } + + raw, ok := cm.Data["platform-os-max-allowed-address-pairs"] + if !ok { + return result, nil + } + + val, err := strconv.Atoi(raw) + if err != nil { + return result, fmt.Errorf("error parsing cloud-network-config platform-os-max-allowed-address-pairs=%q: %w", raw, err) + } + + result.OSMaxAllowedAddressPairs = &val + return result, nil +} diff --git a/pkg/network/bootstrap_test.go b/pkg/network/bootstrap_test.go index 869d78b1cc..6f6ef42492 100644 --- a/pkg/network/bootstrap_test.go +++ b/pkg/network/bootstrap_test.go @@ -215,3 +215,179 @@ func TestBootstrap(t *testing.T) { }) }) } + +func TestBootstrapCloudNetworkConfig(t *testing.T) { + baseOperConfig := &operv1.Network{ + ObjectMeta: metav1.ObjectMeta{Name: names.OPERATOR_CONFIG}, + Spec: operv1.NetworkSpec{ + DefaultNetwork: operv1.DefaultNetworkDefinition{ + Type: operv1.NetworkTypeOVNKubernetes, + OVNKubernetesConfig: &operv1.OVNKubernetesConfig{ + MTU: nil, + }, + }, + }, + } + + baseClientObjs := func(platformType configv1.PlatformType) []crclient.Object { + return []crclient.Object{ + &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: platformType, + }, + }, + }, + &configv1.Proxy{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: network.CLUSTER_CONFIG_NAME, + Namespace: network.CLUSTER_CONFIG_NAMESPACE, + }, + Data: map[string]string{ + "install-config": "controlPlane:\n replicas: 3\n", + }, + }, + &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + }, + } + } + + tests := []struct { + name string + platformType configv1.PlatformType + configMap *corev1.ConfigMap + expectValue *int + expectErr bool + }{ + { + name: "skipped on non-OpenStack platform", + platformType: configv1.NonePlatformType, + configMap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-network-config", + Namespace: "openshift-network-operator", + }, + Data: map[string]string{ + "platform-os-max-allowed-address-pairs": "20", + }, + }, + expectValue: nil, + }, + { + name: "ConfigMap absent", + platformType: configv1.OpenStackPlatformType, + configMap: nil, + expectValue: nil, + }, + { + name: "key missing from ConfigMap", + platformType: configv1.OpenStackPlatformType, + configMap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-network-config", + Namespace: "openshift-network-operator", + }, + Data: map[string]string{"other-key": "value"}, + }, + expectValue: nil, + }, + { + name: "valid value 20", + platformType: configv1.OpenStackPlatformType, + configMap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-network-config", + Namespace: "openshift-network-operator", + }, + Data: map[string]string{ + "platform-os-max-allowed-address-pairs": "20", + }, + }, + expectValue: toPtr(20), + }, + { + name: "zero value", + platformType: configv1.OpenStackPlatformType, + configMap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-network-config", + Namespace: "openshift-network-operator", + }, + Data: map[string]string{ + "platform-os-max-allowed-address-pairs": "0", + }, + }, + expectValue: toPtr(0), + }, + { + name: "negative value", + platformType: configv1.OpenStackPlatformType, + configMap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-network-config", + Namespace: "openshift-network-operator", + }, + Data: map[string]string{ + "platform-os-max-allowed-address-pairs": "-5", + }, + }, + expectValue: toPtr(-5), + }, + { + name: "non-integer value", + platformType: configv1.OpenStackPlatformType, + configMap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-network-config", + Namespace: "openshift-network-operator", + }, + Data: map[string]string{ + "platform-os-max-allowed-address-pairs": "abc", + }, + }, + expectValue: nil, + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + objs := baseClientObjs(tc.platformType) + if tc.configMap != nil { + objs = append(objs, tc.configMap) + } + client := fakeclient.NewFakeClient(objs...) + + result, err := network.Bootstrap(baseOperConfig, client) + if tc.expectErr { + if err == nil { + t.Errorf("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("Bootstrap failed: %v", err) + } + + got := result.CloudNetworkConfig.OSMaxAllowedAddressPairs + switch { + case tc.expectValue == nil && got != nil: + t.Errorf("expected nil, got %d", *got) + case tc.expectValue != nil && got == nil: + t.Errorf("expected %d, got nil", *tc.expectValue) + case tc.expectValue != nil && got != nil && *got != *tc.expectValue: + t.Errorf("expected %d, got %d", *tc.expectValue, *got) + } + }) + } +} + +// Convenience function to create pointers for tests only +func toPtr[t any](i t) *t { + return &i +} diff --git a/pkg/network/cloud_network.go b/pkg/network/cloud_network.go index 389c08aae8..a25c2ce6bd 100644 --- a/pkg/network/cloud_network.go +++ b/pkg/network/cloud_network.go @@ -1,6 +1,7 @@ package network import ( + "fmt" "net" "os" "path/filepath" @@ -49,6 +50,15 @@ func renderCloudNetworkConfigController(conf *operv1.NetworkSpec, bootstrapResul data.Data["PlatformAzureEnvironment"] = "" data.Data["PlatformAWSCAPath"] = "" + cnc := bootstrapResult.CloudNetworkConfig + if cnc.OSMaxAllowedAddressPairs != nil { + if *cnc.OSMaxAllowedAddressPairs <= 0 { + return nil, fmt.Errorf("invalid cloud-network-config: platform-os-max-allowed-address-pairs must be a non-zero, positive integer, got %d", *cnc.OSMaxAllowedAddressPairs) + } + } + + data.Data["OSMaxAllowedAddressPairs"] = cnc.OSMaxAllowedAddressPairs + // AWS and azure allow for funky endpoint overriding. // in different ways, of course. apiurl := "" diff --git a/pkg/network/cloud_network_test.go b/pkg/network/cloud_network_test.go index 2539e14876..723839d2cf 100644 --- a/pkg/network/cloud_network_test.go +++ b/pkg/network/cloud_network_test.go @@ -1,10 +1,23 @@ package network import ( + "context" + "fmt" + "reflect" + "strings" "testing" + configv1 "github.com/openshift/api/config/v1" + operv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/cluster-network-operator/pkg/bootstrap" "github.com/openshift/cluster-network-operator/pkg/render" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + crfake "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func makeManagedControllerRenderData() render.RenderData { @@ -39,6 +52,7 @@ func makeManagedControllerRenderData() render.RenderData { data.Data["AzureManagedCredsPath"] = "" data.Data["AzureManagedSecretProviderClass"] = "" data.Data["GCPCredentialsPath"] = "" + data.Data["OSMaxAllowedAddressPairs"] = (*int)(nil) return data } @@ -171,3 +185,313 @@ func TestCloudTokenMinterHasTokenAudience(t *testing.T) { } t.Fatal("Deployment object not found in rendered output") } + +func makeSelfHostedControllerRenderData() render.RenderData { + data := render.MakeRenderData() + data.Data["ReleaseVersion"] = "4.18.0" + data.Data["PlatformType"] = "OpenStack" + data.Data["PlatformRegion"] = "regionOne" + data.Data["PlatformTypeAWS"] = "AWS" + data.Data["PlatformTypeAzure"] = "Azure" + data.Data["PlatformTypeGCP"] = "GCP" + data.Data["CloudNetworkConfigControllerImage"] = "test-image" + data.Data["KubernetesServiceURL"] = "https://localhost:6443" + data.Data["ExternalControlPlane"] = false + data.Data["PlatformAzureEnvironment"] = "" + data.Data["PlatformAWSCAPath"] = "" + data.Data["PlatformAPIURL"] = "" + data.Data["HTTP_PROXY"] = "" + data.Data["HTTPS_PROXY"] = "" + data.Data["NO_PROXY"] = "" + data.Data["OSMaxAllowedAddressPairs"] = (*int)(nil) + return data +} + +func TestOSMaxAllowedAddressPairsManagedTemplateRendering(t *testing.T) { + tests := []struct { + name string + value *int + expectFlag bool + expectValue string + }{ + { + name: "not set - flag absent", + value: nil, + expectFlag: false, + }, + { + name: "valid value 20 - flag present", + value: toPtr(20), + expectFlag: true, + expectValue: "20", + }, + { + name: "valid value 1 - flag present", + value: toPtr(1), + expectFlag: true, + expectValue: "1", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + data := makeManagedControllerRenderData() + data.Data["OSMaxAllowedAddressPairs"] = tc.value + + objs, err := render.RenderDir("../../bindata/cloud-network-config-controller/managed", &data) + if err != nil { + t.Fatalf("failed to render managed controller: %v", err) + } + + for _, obj := range objs { + if obj.GetKind() != "Deployment" { + continue + } + + container, found := findUnstructuredContainer(t, obj.Object, "controller") + if !found { + t.Fatal("controller container not found in Deployment") + } + + cmdSlice, found, err := uns.NestedStringSlice(container, "command") + if err != nil || !found || len(cmdSlice) < 3 { + t.Fatal("command not found in controller container") + } + shellScript := cmdSlice[2] + + flagStr := fmt.Sprintf("-platform-os-max-allowed-address-pairs=%s", tc.expectValue) + if tc.expectFlag && !strings.Contains(shellScript, flagStr) { + t.Errorf("expected shell script to contain %q, but it does not.\nScript:\n%s", flagStr, shellScript) + } + if !tc.expectFlag && strings.Contains(shellScript, "-platform-os-max-allowed-address-pairs=") { + t.Errorf("expected shell script to NOT contain the flag, but it does.\nScript:\n%s", shellScript) + } + return + } + t.Fatal("Deployment object not found in rendered output") + }) + } +} + +func TestOSMaxAllowedAddressPairsSelfHostedTemplateRendering(t *testing.T) { + tests := []struct { + name string + value *int + expectFlag bool + expectValue string + }{ + { + name: "not set - flag absent", + value: nil, + expectFlag: false, + }, + { + name: "valid value 20 - flag present", + value: toPtr(20), + expectFlag: true, + expectValue: "20", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + data := makeSelfHostedControllerRenderData() + data.Data["OSMaxAllowedAddressPairs"] = tc.value + + objs, err := render.RenderDir("../../bindata/cloud-network-config-controller/self-hosted", &data) + if err != nil { + t.Fatalf("failed to render self-hosted controller: %v", err) + } + + for _, obj := range objs { + if obj.GetKind() != "Deployment" { + continue + } + + container, found := findUnstructuredContainer(t, obj.Object, "controller") + if !found { + t.Fatal("controller container not found in Deployment") + } + + args, found, err := uns.NestedStringSlice(container, "args") + if err != nil || !found { + t.Fatal("args not found in controller container") + } + + flagStr := fmt.Sprintf("-platform-os-max-allowed-address-pairs=%s", tc.expectValue) + var hasFlag bool + for _, arg := range args { + if tc.expectFlag && arg == flagStr { + hasFlag = true + break + } + if !tc.expectFlag && strings.Contains(arg, "-platform-os-max-allowed-address-pairs=") { + t.Errorf("expected args to NOT contain the flag, but found %q", arg) + return + } + } + if tc.expectFlag && !hasFlag { + t.Errorf("expected args to contain %q, but it was not found.\nArgs: %v", flagStr, args) + } + return + } + t.Fatal("Deployment object not found in rendered output") + }) + } +} + +func TestOSMaxAllowedAddressPairsRenderValidation(t *testing.T) { + tests := []struct { + name string + value *int + expectErr bool + errSubstr string + }{ + { + name: "zero when set - returns error", + value: toPtr(0), + expectErr: true, + errSubstr: "got 0", + }, + { + name: "negative when set - returns error", + value: toPtr(-5), + expectErr: true, + errSubstr: "got -5", + }, + { + name: "valid positive value - no error", + value: toPtr(20), + expectErr: false, + }, + { + name: "not set - no error", + value: nil, + expectErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + conf := &operv1.NetworkSpec{ + DefaultNetwork: operv1.DefaultNetworkDefinition{ + Type: operv1.NetworkTypeOVNKubernetes, + }, + } + br := &bootstrap.BootstrapResult{ + Infra: bootstrap.InfraStatus{ + PlatformType: configv1.OpenStackPlatformType, + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.OpenStackPlatformType, + }, + APIServers: map[string]bootstrap.APIServer{ + bootstrap.APIServerDefault: {Host: "localhost", Port: "6443"}, + bootstrap.APIServerDefaultLocal: {Host: "localhost", Port: "6443"}, + }, + KubeCloudConfig: map[string]string{}, + }, + CloudNetworkConfig: bootstrap.CloudNetworkConfigBootstrapResult{ + OSMaxAllowedAddressPairs: tc.value, + }, + } + + _, err := renderCloudNetworkConfigController(conf, br, "../../bindata") + if tc.expectErr && err == nil { + t.Error("expected error, got nil") + } + if tc.expectErr && err != nil && tc.errSubstr != "" && !strings.Contains(err.Error(), tc.errSubstr) { + t.Errorf("expected error to contain %q, got: %v", tc.errSubstr, err) + } + if !tc.expectErr && err != nil { + t.Errorf("expected no error, got: %v", err) + } + }) + } +} + +type erroringReader struct { + err error +} + +func (r *erroringReader) Get(_ context.Context, _ crclient.ObjectKey, _ crclient.Object, _ ...crclient.GetOption) error { + return r.err +} + +func (r *erroringReader) List(_ context.Context, _ crclient.ObjectList, _ ...crclient.ListOption) error { + return r.err +} + +func TestCloudNetworkConfigBootstrapErrorPropagation(t *testing.T) { + tests := []struct { + name string + reader crclient.Reader + expectErr bool + errSubstr string + expectRes bootstrap.CloudNetworkConfigBootstrapResult + }{ + { + name: "transient API error is propagated", + reader: &erroringReader{err: fmt.Errorf("connection refused")}, + expectErr: true, + errSubstr: "connection refused", + }, + { + name: "forbidden error is propagated", + reader: &erroringReader{err: apierrors.NewForbidden( + schema.GroupResource{Group: "", Resource: "configmaps"}, "cloud-network-config", fmt.Errorf("access denied"), + )}, + expectErr: true, + errSubstr: "forbidden", + }, + { + name: "not-found error returns empty result without error", + reader: &erroringReader{err: apierrors.NewNotFound( + schema.GroupResource{Group: "", Resource: "configmaps"}, "cloud-network-config", + )}, + expectErr: false, + expectRes: bootstrap.CloudNetworkConfigBootstrapResult{}, + }, + { + name: "ConfigMap present with valid value succeeds", + reader: crfake.NewClientBuilder().WithObjects(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-network-config", + Namespace: "openshift-network-operator", + }, + Data: map[string]string{ + "platform-os-max-allowed-address-pairs": "15", + }, + }).Build(), + expectErr: false, + expectRes: bootstrap.CloudNetworkConfigBootstrapResult{ + OSMaxAllowedAddressPairs: toPtr(15), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result, err := cloudNetworkConfigBootstrap(context.Background(), tc.reader) + if tc.expectErr { + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), tc.errSubstr) { + t.Errorf("expected error containing %q, got: %v", tc.errSubstr, err) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !reflect.DeepEqual(result, tc.expectRes) { + t.Errorf("result mismatch:\n got: %+v\n want: %+v", result, tc.expectRes) + } + }) + } +} + +// Convenience function to create pointers for tests only +func toPtr[t any](i t) *t { + return &i +}