OCPBUGS-111994: Add --frrk8s-secret-passthrough for external frr-k8s

charts/metallb/README.md

@@ -64,6 +64,7 @@ Kubernetes: `>= 1.19.0-0`
 | frrk8s.enabled | bool | `false` |  |
 | frrk8s.external | bool | `false` |  |
 | frrk8s.namespace | string | `""` |  |
+| frrk8s.secretPassthrough | bool | `false` | Pass BGP secret references to frr-k8s without resolving them. The secret must exist in the frr-k8s namespace. Only used when external=true. |
 | fullnameOverride | string | `""` |  |
 | imagePullSecrets | list | `[]` |  |
 | loadBalancerClass | string | `""` |  |

charts/metallb/templates/speaker.yaml

@@ -273,6 +273,9 @@ spec:
         {{- end }}
         {{- if .Values.frrk8s.external }}
         - --frrk8s-namespace={{ required "namespace is required when frrk8s is external" .Values.frrk8s.namespace }}
+        {{- if .Values.frrk8s.secretPassthrough }}
+        - --frrk8s-secret-passthrough
+        {{- end }}
         {{- end }}
         env:
         - name: METALLB_NODE_NAME

charts/metallb/values.yaml

@@ -376,6 +376,9 @@ frrk8s:
   enabled: false
   external: false
   namespace: ""
+  # -- Pass BGP secret references to frr-k8s without resolving them. The secret must
+  # exist in the frr-k8s namespace. Only used when external=true.
+  secretPassthrough: false
 
 # networkpolicies
 networkpolicies:

configmaptocrs/main.go

@@ -97,7 +97,7 @@ func generate(w io.Writer, origin string) error {
 	}
 
 	log.Println("Checking the resources are parsed correctly")
-	_, err = config.For(resources, config.DontValidate)
+	_, err = config.For(resources, config.DontValidate, config.ForOptions{})
 	if err != nil {
 		return err
 	}

internal/config/config.go

@@ -52,6 +52,14 @@ type ClusterResources struct {
 	BGPExtras       corev1.ConfigMap                  `json:"bgpextras"`
 }
 
+// ForOptions holds optional parameters for the For config parser.
+type ForOptions struct {
+	// FRRK8sSecretPassthrough skips resolving password secrets locally and passes
+	// the secret reference as-is to frr-k8s. Used when frr-k8s runs in a separate
+	// namespace and the secret is created there directly.
+	FRRK8sSecretPassthrough bool
+}
+
 // Config is a parsed MetalLB configuration.
 type Config struct {
 	// Routers that MetalLB should peer with.
@@ -228,7 +236,7 @@ func (p *Pools) IsEmpty(pool string) bool {
 }
 
 // Parse loads and validates a Config from bs.
-func For(resources ClusterResources, validate Validate) (*Config, error) {
+func For(resources ClusterResources, validate Validate, opts ForOptions) (*Config, error) {
 	err := validate(resources)
 	if err != nil {
 		return nil, err
@@ -241,7 +249,7 @@ func For(resources ClusterResources, validate Validate) (*Config, error) {
 		return nil, err
 	}
 
-	cfg.Peers, err = peersFor(resources, cfg.BFDProfiles)
+	cfg.Peers, err = peersFor(resources, cfg.BFDProfiles, opts)
 	if err != nil {
 		return nil, err
 	}
@@ -278,10 +286,10 @@ func bfdProfilesFor(resources ClusterResources) (map[string]*BFDProfile, error)
 	return res, nil
 }
 
-func peersFor(resources ClusterResources, BFDProfiles map[string]*BFDProfile) (map[string]*Peer, error) {
+func peersFor(resources ClusterResources, BFDProfiles map[string]*BFDProfile, opts ForOptions) (map[string]*Peer, error) {
 	var res = make(map[string]*Peer)
 	for _, p := range resources.Peers {
-		peer, err := peerFromCR(p, resources.PasswordSecrets)
+		peer, err := peerFromCR(p, resources.PasswordSecrets, opts.FRRK8sSecretPassthrough)
 		if err != nil {
 			return nil, fmt.Errorf("parsing peer %s %w", p.Name, err)
 		}
@@ -378,7 +386,7 @@ func communitiesFromCrs(cs []metallbv1beta1.Community) (map[string]community.BGP
 	return communities, nil
 }
 
-func peerFromCR(p metallbv1beta2.BGPPeer, passwordSecrets map[string]corev1.Secret) (*Peer, error) {
+func peerFromCR(p metallbv1beta2.BGPPeer, passwordSecrets map[string]corev1.Secret, frrk8sSecretPassthrough bool) (*Peer, error) {
 	if p.Spec.MyASN == 0 {
 		return nil, errors.New("missing local ASN")
 	}
@@ -452,7 +460,7 @@ func peerFromCR(p metallbv1beta2.BGPPeer, passwordSecrets map[string]corev1.Secr
 	}
 
 	secretPassword := ""
-	if p.Spec.PasswordSecret.Name != "" {
+	if p.Spec.PasswordSecret.Name != "" && !frrk8sSecretPassthrough {
 		secretPassword, err = passwordFromSecretForPeer(p, passwordSecrets)
 		if err != nil {
 			return nil, errors.Join(err, fmt.Errorf("failed to parse peer %s password secret", p.Name))

internal/config/config_test.go

@@ -48,6 +48,7 @@ func TestParse(t *testing.T) {
 	tests := []struct {
 		desc string
 		crs  ClusterResources
+		opts ForOptions
 		want *Config
 	}{
 		{
@@ -2227,6 +2228,88 @@ func TestParse(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "BGP Peer with secret ref and frrk8s secret passthrough skips resolution even when secret exists",
+			crs: ClusterResources{
+				Peers: []v1beta2.BGPPeer{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "peer1",
+						},
+						Spec: v1beta2.BGPPeerSpec{
+							MyASN:   42,
+							ASN:     42,
+							Port:    179,
+							Address: "1.2.3.4",
+							PasswordSecret: corev1.SecretReference{Name: "bgpsecret",
+								Namespace: "frr-k8s-external"},
+						},
+					},
+				},
+				PasswordSecrets: map[string]corev1.Secret{
+					"bgpsecret": {Type: corev1.SecretTypeBasicAuth, ObjectMeta: metav1.ObjectMeta{Name: "bgpsecret", Namespace: "metallb-system"},
+						Data: map[string][]byte{"password": []byte("shouldnotresolve")}},
+				},
+			},
+			opts: ForOptions{FRRK8sSecretPassthrough: true},
+			want: &Config{
+				Peers: map[string]*Peer{
+					"peer1": {
+						Name:          "peer1",
+						MyASN:         42,
+						ASN:           42,
+						Addr:          net.ParseIP("1.2.3.4"),
+						Port:          179,
+						NodeSelectors: []labels.Selector{labels.Everything()},
+						PasswordRef: corev1.SecretReference{
+							Name:      "bgpsecret",
+							Namespace: "frr-k8s-external",
+						},
+					},
+				},
+				Pools:       &Pools{ByName: map[string]*Pool{}},
+				BFDProfiles: map[string]*BFDProfile{},
+			},
+		},
+		{
+			desc: "BGP Peer with secret ref and frrk8s secret passthrough",
+			crs: ClusterResources{
+				Peers: []v1beta2.BGPPeer{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "peer1",
+						},
+						Spec: v1beta2.BGPPeerSpec{
+							MyASN:   42,
+							ASN:     42,
+							Port:    179,
+							Address: "1.2.3.4",
+							PasswordSecret: corev1.SecretReference{Name: "bgpsecret",
+								Namespace: "frr-k8s-external"},
+						},
+					},
+				},
+			},
+			opts: ForOptions{FRRK8sSecretPassthrough: true},
+			want: &Config{
+				Peers: map[string]*Peer{
+					"peer1": {
+						Name:          "peer1",
+						MyASN:         42,
+						ASN:           42,
+						Addr:          net.ParseIP("1.2.3.4"),
+						Port:          179,
+						NodeSelectors: []labels.Selector{labels.Everything()},
+						PasswordRef: corev1.SecretReference{
+							Name:      "bgpsecret",
+							Namespace: "frr-k8s-external",
+						},
+					},
+				},
+				Pools:       &Pools{ByName: map[string]*Pool{}},
+				BFDProfiles: map[string]*BFDProfile{},
+			},
+		},
 		{
 			desc: "Peer with non existing BFD Profile",
 			crs: ClusterResources{
@@ -3670,7 +3753,7 @@ func TestParse(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			got, err := For(test.crs, DontValidate)
+			got, err := For(test.crs, DontValidate, test.opts)
 			if err != nil && test.want != nil {
 				t.Errorf("%q: parse failed: %s", test.desc, err)
 				return

internal/config/validator.go

@@ -56,7 +56,7 @@ func (v *validator) Validate(resources ...client.ObjectList) error {
 		}
 	}
 	clusterResources = resetTransientErrorsFields(clusterResources)
-	_, err := For(clusterResources, v.validate)
+	_, err := For(clusterResources, v.validate, ForOptions{})
 	return err
 }
 

internal/k8s/controllers/config_controller.go

@@ -40,14 +40,15 @@ const bgpExtrasConfigName = "bgpextras"
 
 type ConfigReconciler struct {
 	client.Client
-	Logger         log.Logger
-	Scheme         *runtime.Scheme
-	Namespace      string
-	Handler        func(log.Logger, *config.Config) SyncState
-	ValidateConfig config.Validate
-	ForceReload    func()
-	BGPType        string
-	currentConfig  *config.Config
+	Logger                  log.Logger
+	Scheme                  *runtime.Scheme
+	Namespace               string
+	Handler                 func(log.Logger, *config.Config) SyncState
+	ValidateConfig          config.Validate
+	ForceReload             func()
+	BGPType                 string
+	currentConfig           *config.Config
+	FRRK8sSecretPassthrough bool
 }
 
 func (r *ConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
@@ -134,7 +135,9 @@ var requestHandler = func(r *ConfigReconciler, ctx context.Context, req ctrl.Req
 
 	level.Debug(r.Logger).Log("controller", "ConfigReconciler", "metallb CRs and Secrets", dumpClusterResources(&resources))
 
-	cfg, err := toConfig(resources, r.ValidateConfig)
+	cfg, err := toConfig(resources, r.ValidateConfig, config.ForOptions{
+		FRRK8sSecretPassthrough: r.FRRK8sSecretPassthrough,
+	})
 	if err != nil {
 		configStale.Set(1)
 		level.Error(r.Logger).Log("controller", "ConfigReconciler", "error", "failed to parse the configuration", "error", err)

internal/k8s/controllers/config_controller_test.go

@@ -91,7 +91,7 @@ func TestConfigController(t *testing.T) {
 			t.Fatalf("test %s failed to create fake client: %v", test.desc, err)
 		}
 
-		expectedCfg, err := config.For(resources, config.DontValidate)
+		expectedCfg, err := config.For(resources, config.DontValidate, config.ForOptions{})
 		if err != nil && test.validResources {
 			t.Fatalf("test %s failed to create config, got unexpected error: %v", test.desc, err)
 		}

internal/k8s/controllers/config_conversion.go

@@ -8,7 +8,7 @@ import (
 	"go.universe.tf/metallb/internal/config"
 )
 
-func toConfig(fromK8s config.ClusterResources, validate config.Validate) (*config.Config, error) {
+func toConfig(fromK8s config.ClusterResources, validate config.Validate, opts config.ForOptions) (*config.Config, error) {
 	resources := config.ClusterResources{
 		Pools:           sortedCopy(fromK8s.Pools),
 		Peers:           sortedCopy(fromK8s.Peers),
@@ -22,7 +22,7 @@ func toConfig(fromK8s config.ClusterResources, validate config.Validate) (*confi
 		BGPExtras:       fromK8s.BGPExtras,
 	}
 
-	cfg, err := config.For(resources, validate)
+	cfg, err := config.For(resources, validate, opts)
 	return cfg, err
 }
 

internal/k8s/controllers/config_conversion_test.go

@@ -189,7 +189,7 @@ func TestConversionIsStable(t *testing.T) {
 		Namespaces:      namespaces,
 	}
 
-	firstConfig, err := toConfig(resources, config.DontValidate)
+	firstConfig, err := toConfig(resources, config.DontValidate, config.ForOptions{})
 
 	if err != nil {
 		t.Fatalf("conversion failed, err %v", err)
@@ -209,7 +209,7 @@ func TestConversionIsStable(t *testing.T) {
 		shuffleObjects(resources.Nodes)
 		shuffleObjects(resources.Namespaces)
 
-		config, err := toConfig(resources, config.DontValidate)
+		config, err := toConfig(resources, config.DontValidate, config.ForOptions{})
 
 		if err != nil {
 			t.Fatalf("conversion failed, seed %d, %v", seed, err)

internal/k8s/controllers/pool_controller.go

@@ -75,7 +75,7 @@ func (r *PoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 	level.Debug(r.Logger).Log("controller", "PoolReconciler", "metallb CRs", dumpClusterResources(&resources))
 
-	cfg, err := toConfig(resources, r.ValidateConfig)
+	cfg, err := toConfig(resources, r.ValidateConfig, config.ForOptions{})
 	if err != nil {
 		configStale.Set(1)
 		level.Error(r.Logger).Log("controller", "PoolReconciler", "error", "failed to parse the configuration", "error", err)

internal/k8s/controllers/pool_controller_test.go

@@ -89,7 +89,7 @@ func TestPoolController(t *testing.T) {
 			t.Fatalf("test %s failed to create fake client: %v", test.desc, err)
 		}
 
-		expectedCfg, err := metallbcfg.For(resources, metallbcfg.DontValidate)
+		expectedCfg, err := metallbcfg.For(resources, metallbcfg.DontValidate, metallbcfg.ForOptions{})
 		if err != nil && test.validResources {
 			t.Fatalf("test %s failed to create config, got unexpected error: %v", test.desc, err)
 		}

internal/k8s/k8s.go

@@ -98,27 +98,28 @@ type Client struct {
 // Config specifies the configuration of the Kubernetes
 // client/watcher.
 type Config struct {
-	ProcessName         string
-	NodeName            string
-	PodName             string
-	MetricsHost         string
-	MetricsPort         int
-	EnablePprof         bool
-	ReadEndpoints       bool
-	Logger              log.Logger
-	Namespace           string
-	ValidateConfig      config.Validate
-	EnableWebhook       bool
-	WebHookMinVersion   uint16
-	WebHookCipherSuites []uint16
-	DisableCertRotation bool
-	WebhookSecretName   string
-	CertDir             string
-	CertServiceName     string
-	LoadBalancerClass   string
-	WebhookWithHTTP2    bool
-	WithFRRK8s          bool
-	FRRK8sNamespace     string
+	ProcessName             string
+	NodeName                string
+	PodName                 string
+	MetricsHost             string
+	MetricsPort             int
+	EnablePprof             bool
+	ReadEndpoints           bool
+	Logger                  log.Logger
+	Namespace               string
+	ValidateConfig          config.Validate
+	EnableWebhook           bool
+	WebHookMinVersion       uint16
+	WebHookCipherSuites     []uint16
+	DisableCertRotation     bool
+	WebhookSecretName       string
+	CertDir                 string
+	CertServiceName         string
+	LoadBalancerClass       string
+	WebhookWithHTTP2        bool
+	WithFRRK8s              bool
+	FRRK8sNamespace         string
+	FRRK8sSecretPassthrough bool
 	Listener
 	Layer2StatusChan    <-chan event.GenericEvent
 	Layer2StatusFetcher controllers.L2StatusFetcher
@@ -188,13 +189,14 @@ func New(cfg *Config) (*Client, error) {
 
 	if cfg.ConfigChanged != nil {
 		if err = (&controllers.ConfigReconciler{
-			Client:         mgr.GetClient(),
-			Logger:         cfg.Logger,
-			Scheme:         mgr.GetScheme(),
-			Namespace:      cfg.Namespace,
-			ValidateConfig: cfg.ValidateConfig,
-			Handler:        cfg.ConfigHandler,
-			ForceReload:    reload,
+			Client:                  mgr.GetClient(),
+			Logger:                  cfg.Logger,
+			Scheme:                  mgr.GetScheme(),
+			Namespace:               cfg.Namespace,
+			ValidateConfig:          cfg.ValidateConfig,
+			Handler:                 cfg.ConfigHandler,
+			ForceReload:             reload,
+			FRRK8sSecretPassthrough: cfg.FRRK8sSecretPassthrough,
 		}).SetupWithManager(mgr); err != nil {
 			level.Error(c.logger).Log("error", err, "unable to create controller", "config")
 			return nil, errors.Join(err, errors.New("unable to create controller for config"))