Merge pull request #80 from oribon/add_password

Add cleartext password support

API-DOCS.md

@@ -229,7 +229,8 @@ _Appears in:_
 | `asn` _integer_ | ASN is the AS number to use for the local end of the session. |
 | `address` _string_ | Address is the IP address to establish the session with. |
 | `port` _integer_ | Port is the port to dial when establishing the session. Defaults to 179. |
-| `password` _[SecretReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#secretreference-v1-core)_ | PasswordSecret is name of the authentication secret for the neighbor. the secret must be of type "kubernetes.io/basic-auth", and created in the same namespace as the frr-k8s daemon. The password is stored in the secret as the key "password". |
+| `password` _string_ | Password to be used for establishing the BGP session. Password and PasswordSecret are mutually exclusive. |
+| `passwordSecret` _[SecretReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#secretreference-v1-core)_ | PasswordSecret is name of the authentication secret for the neighbor. the secret must be of type "kubernetes.io/basic-auth", and created in the same namespace as the frr-k8s daemon. The password is stored in the secret as the key "password". Password and PasswordSecret are mutually exclusive. |
 | `holdTime` _[Duration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#duration-v1-meta)_ | HoldTime is the requested BGP hold time, per RFC4271. Defaults to 180s. |
 | `keepaliveTime` _[Duration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#duration-v1-meta)_ | KeepaliveTime is the requested BGP keepalive time, per RFC4271. Defaults to 60s. |
 | `ebgpMultiHop` _boolean_ | EBGPMultiHop indicates if the BGPPeer is multi-hops away. |

api/v1beta1/frrconfiguration_types.go

@@ -99,12 +99,18 @@ type Neighbor struct {
 	// +kubebuilder:validation:Maximum=16384
 	Port *uint16 `json:"port,omitempty"`
 
+	// Password to be used for establishing the BGP session.
+	// Password and PasswordSecret are mutually exclusive.
+	// +optional
+	Password string `json:"password,omitempty"`
+
 	// PasswordSecret is name of the authentication secret for the neighbor.
 	// the secret must be of type "kubernetes.io/basic-auth", and created in the
 	// same namespace as the frr-k8s daemon. The password is stored in the
 	// secret as the key "password".
+	// Password and PasswordSecret are mutually exclusive.
 	// +optional
-	PasswordSecret v1.SecretReference `json:"password,omitempty"`
+	PasswordSecret v1.SecretReference `json:"passwordSecret,omitempty"`
 
 	// HoldTime is the requested BGP hold time, per RFC4271.
 	// Defaults to 180s.

charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrconfigurations.yaml

@@ -157,11 +157,17 @@ spec:
                                   time, per RFC4271. Defaults to 60s.
                                 type: string
                               password:
+                                description: Password to be used for establishing
+                                  the BGP session. Password and PasswordSecret are
+                                  mutually exclusive.
+                                type: string
+                              passwordSecret:
                                 description: PasswordSecret is name of the authentication
                                   secret for the neighbor. the secret must be of type
                                   "kubernetes.io/basic-auth", and created in the same
                                   namespace as the frr-k8s daemon. The password is
-                                  stored in the secret as the key "password".
+                                  stored in the secret as the key "password". Password
+                                  and PasswordSecret are mutually exclusive.
                                 properties:
                                   name:
                                     description: name is unique within a namespace

config/all-in-one/frr-k8s-prometheus.yaml

@@ -172,11 +172,17 @@ spec:
                                   time, per RFC4271. Defaults to 60s.
                                 type: string
                               password:
+                                description: Password to be used for establishing
+                                  the BGP session. Password and PasswordSecret are
+                                  mutually exclusive.
+                                type: string
+                              passwordSecret:
                                 description: PasswordSecret is name of the authentication
                                   secret for the neighbor. the secret must be of type
                                   "kubernetes.io/basic-auth", and created in the same
                                   namespace as the frr-k8s daemon. The password is
-                                  stored in the secret as the key "password".
+                                  stored in the secret as the key "password". Password
+                                  and PasswordSecret are mutually exclusive.
                                 properties:
                                   name:
                                     description: name is unique within a namespace

config/all-in-one/frr-k8s.yaml

@@ -172,11 +172,17 @@ spec:
                                   time, per RFC4271. Defaults to 60s.
                                 type: string
                               password:
+                                description: Password to be used for establishing
+                                  the BGP session. Password and PasswordSecret are
+                                  mutually exclusive.
+                                type: string
+                              passwordSecret:
                                 description: PasswordSecret is name of the authentication
                                   secret for the neighbor. the secret must be of type
                                   "kubernetes.io/basic-auth", and created in the same
                                   namespace as the frr-k8s daemon. The password is
-                                  stored in the secret as the key "password".
+                                  stored in the secret as the key "password". Password
+                                  and PasswordSecret are mutually exclusive.
                                 properties:
                                   name:
                                     description: name is unique within a namespace

config/crd/bases/frrk8s.metallb.io_frrconfigurations.yaml

@@ -157,11 +157,17 @@ spec:
                                   time, per RFC4271. Defaults to 60s.
                                 type: string
                               password:
+                                description: Password to be used for establishing
+                                  the BGP session. Password and PasswordSecret are
+                                  mutually exclusive.
+                                type: string
+                              passwordSecret:
                                 description: PasswordSecret is name of the authentication
                                   secret for the neighbor. the secret must be of type
                                   "kubernetes.io/basic-auth", and created in the same
                                   namespace as the frr-k8s daemon. The password is
-                                  stored in the secret as the key "password".
+                                  stored in the secret as the key "password". Password
+                                  and PasswordSecret are mutually exclusive.
                                 properties:
                                   name:
                                     description: name is unique within a namespace

config/samples/neighbor_with_password.yaml

@@ -11,7 +11,7 @@ spec:
       - address: 172.30.0.3
         asn: 4200000000
         ebgpMultiHop: true
-        password:
+        passwordSecret:
           name: passwordsecret
           namespace: frr-k8s-system
         port: 180

e2etests/tests/session.go

@@ -17,6 +17,8 @@ import (
 	"go.universe.tf/e2etest/pkg/executor"
 	"go.universe.tf/e2etest/pkg/frr"
 	frrconfig "go.universe.tf/e2etest/pkg/frr/config"
+	frrcontainer "go.universe.tf/e2etest/pkg/frr/container"
+	"go.universe.tf/e2etest/pkg/ipfamily"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
@@ -114,5 +116,65 @@ var _ = ginkgo.Describe("Advertisement", func() {
 				}, 2*time.Minute, time.Second).ShouldNot(HaveOccurred())
 			}
 		})
+
+		ginkgo.DescribeTable("Establishes sessions with cleartext password", func(family ipfamily.Family) {
+			frrs := config.ContainersForVRF(infra.FRRContainers, "")
+			neighbors := []frrk8sv1beta1.Neighbor{}
+
+			for _, f := range frrs {
+				addresses := f.AddressesForFamily(family)
+				ebgpMultihop := false
+				if f.NeighborConfig.MultiHop && f.NeighborConfig.ASN != f.RouterConfig.ASN {
+					ebgpMultihop = true
+				}
+
+				for _, address := range addresses {
+					neighbors = append(neighbors, frrk8sv1beta1.Neighbor{
+						ASN:          f.RouterConfig.ASN,
+						Address:      address,
+						Password:     f.RouterConfig.Password,
+						Port:         &f.RouterConfig.BGPPort,
+						EBGPMultiHop: ebgpMultihop,
+					})
+				}
+			}
+
+			config := frrk8sv1beta1.FRRConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: k8s.FRRK8sNamespace,
+				},
+				Spec: frrk8sv1beta1.FRRConfigurationSpec{
+					BGP: frrk8sv1beta1.BGPConfig{
+						Routers: []frrk8sv1beta1.Router{
+							{
+								ASN:       infra.FRRK8sASN,
+								VRF:       "",
+								Neighbors: neighbors,
+							},
+						},
+					},
+				},
+			}
+
+			ginkgo.By("pairing with nodes")
+			for _, c := range frrs {
+				err := frrcontainer.PairWithNodes(cs, c, family)
+				framework.ExpectNoError(err)
+			}
+
+			err := updater.Update([]corev1.Secret{}, config)
+			framework.ExpectNoError(err)
+
+			nodes, err := k8s.Nodes(cs)
+			framework.ExpectNoError(err)
+
+			for _, c := range frrs {
+				ValidateFRRPeeredWithNodes(nodes, c, family)
+			}
+		},
+			ginkgo.Entry("IPV4", ipfamily.IPv4),
+			ginkgo.Entry("IPV6", ipfamily.IPv6),
+		)
 	})
 })

internal/controller/api_to_config.go

@@ -165,9 +165,18 @@ func neighborToFRR(n v1beta1.Neighbor, ipv4Prefixes, ipv6Prefixes []string, rout
 }
 
 func passwordForNeighbor(n v1beta1.Neighbor, passwordSecrets map[string]corev1.Secret) (string, error) {
+	if n.Password != "" && n.PasswordSecret.Name != "" {
+		return "", fmt.Errorf("neighbor %s specifies both cleartext password and secret ref", neighborName(n.ASN, n.Address))
+	}
+
+	if n.Password != "" {
+		return n.Password, nil
+	}
+
 	if n.PasswordSecret.Name == "" {
 		return "", nil
 	}
+
 	secret, ok := passwordSecrets[n.PasswordSecret.Name]
 	if !ok {
 		return "", TransientError{Message: fmt.Sprintf("secret %s not found for neighbor %s", n.PasswordSecret.Name, neighborName(n.ASN, n.Address))}

internal/controller/api_to_config_test.go

@@ -1633,6 +1633,11 @@ func TestConversion(t *testing.T) {
 												Namespace: "frr-k8s-system",
 											},
 										},
+										{
+											ASN:      65012,
+											Address:  "192.0.2.8",
+											Password: "cleartext-password",
+										},
 									},
 									VRF: "",
 								},
@@ -1695,6 +1700,21 @@ func TestConversion(t *testing.T) {
 									PrefixesV6: []frr.IncomingFilter{},
 								},
 							},
+							{
+								IPFamily: ipfamily.IPv4,
+								Name:     "[email protected]",
+								ASN:      65012,
+								Addr:     "192.0.2.8",
+								Password: "cleartext-password",
+								Outgoing: frr.AllowedOut{
+									PrefixesV4: []frr.OutgoingFilter{},
+									PrefixesV6: []frr.OutgoingFilter{},
+								},
+								Incoming: frr.AllowedIn{
+									PrefixesV4: []frr.IncomingFilter{},
+									PrefixesV6: []frr.IncomingFilter{},
+								},
+							},
 						},
 						VRF:          "",
 						IPV4Prefixes: []string{},
@@ -1783,6 +1803,45 @@ func TestConversion(t *testing.T) {
 			expected: nil,
 			err:      errors.New("failed to process neighbor [email protected] for router 65010-: secret ref not found for neighbor [email protected]"),
 		},
+		{
+			name: "Specifying both cleartext password and secret ref",
+			fromK8s: []v1beta1.FRRConfiguration{
+				{
+					Spec: v1beta1.FRRConfigurationSpec{
+						BGP: v1beta1.BGPConfig{
+							Routers: []v1beta1.Router{
+								{
+									ASN: 65010,
+									ID:  "192.0.2.5",
+									Neighbors: []v1beta1.Neighbor{
+										{
+											ASN:      65012,
+											Address:  "192.0.2.7",
+											Password: "cleartext-password",
+											PasswordSecret: v1.SecretReference{
+												Name:      "secret1",
+												Namespace: "frr-k8s-system",
+											},
+										},
+									},
+									VRF: "",
+								},
+							},
+						},
+					},
+				},
+			},
+			secrets: map[string]v1.Secret{
+				"secret1": {
+					Type: v1.SecretTypeBasicAuth,
+					Data: map[string][]byte{
+						"password": []byte("password"),
+					},
+				},
+			},
+			expected: nil,
+			err:      errors.New("failed to process neighbor [email protected] for router 65010-: neighbor [email protected] specifies both cleartext password and secret ref"),
+		},
 		{
 			name: "Single Router and injection",
 			fromK8s: []v1beta1.FRRConfiguration{