Handle passwords #37
Handle passwords #37
Conversation
cmd/main.go
Outdated
| ) | ||
|
|
||
| flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") | ||
| flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") | ||
| flag.StringVar(&logLevel, "log-level", "info", fmt.Sprintf("log level. must be one of: [%s]", logging.Levels.String())) | ||
| flag.StringVar(&nodeName, "node-name", "", "The node this daemon is running on.") | ||
| flag.StringVar(&namespace, "namespace", "", "The namespace this demon is deployed in") |
config/rbac/role_binding.yaml
Outdated
There was a problem hiding this comment.
nit: I think this belongs better to the next commit
internal/controller/api_to_config.go
Outdated
| } | ||
| srcPass, ok := secret.Data["password"] | ||
| if !ok { | ||
| return "", fmt.Errorf("password not specified in the secret %q/%q", secret.Namespace, secret.Name) |
There was a problem hiding this comment.
nit: password -> password field?
| } | ||
|
|
||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| frr, err := apiToFRR(test.fromK8s) | ||
| frr, err := apiToFRR(test.fromK8s, test.secrets) | ||
| if test.err != nil && err == nil { |
There was a problem hiding this comment.
unrelated question: should we start comparing test.err and err when != nil?
There was a problem hiding this comment.
either that or just "expect err" as bool. Let me do that in a separate commit
There was a problem hiding this comment.
is there a reason we don't add a Watches for secrets?
There was a problem hiding this comment.
oops. Adding a test too
config/rbac/secrets_role.yaml
Outdated
There was a problem hiding this comment.
does it make sense to append this to the existing role.yaml instead of a different file?
There was a problem hiding this comment.
nope, because the current role is generated by kubebuilder and gets overridden. And given the controller is mixed (mostly cluster scoped, secrets namespace scoped) kubebuilder generates a cluster role (which we don't want, as we don't want the controller to receive and read all the secrets but only the local ones)
| corev1 "k8s.io/api/core/v1" | ||
| v1 "k8s.io/api/core/v1" |
| @@ -40,14 +53,32 @@ func PeersForContainers(frrs []*frrcontainer.FRR, ipFam ipfamily.Family, modify | |||
| FRR: *f, | |||
| } | |||
|
|
|||
| if f.RouterConfig.Password != "" { | |||
|
Also, there's the |
fedepaol
force-pushed
the
passwordsecrets
branch
from
62324a1
to
fc296ba
Compare
done |
fedepaol
force-pushed
the
passwordsecrets
branch
6 times, most recently
from
5636a3e
to
bf55891
Compare
| corev1 "k8s.io/api/core/v1" | ||
| v1 "k8s.io/api/core/v1" |
|
|
||
| config, err := apiToFRR(cfgs, secrets) | ||
| secretNotFound := SecretNotFoundError{} | ||
| if errors.As(err, &secretNotFound) { // This might be a receiving order issue so we need to return the error and retry |
There was a problem hiding this comment.
is this necessary now that we listen for secret events?
There was a problem hiding this comment.
I'd rather be on the paranoid side. It should converge anyway
|
also, multiple configs for the same neighbor specifying a different password should be considered invalid? |
you already added the check frr-k8s/internal/controller/merge.go Line 215 in 7ecf546 |
fedepaol
force-pushed
the
passwordsecrets
branch
from
bf55891
to
c0fecdf
Compare
We allow users to configure the passwords of a given bgp session using secrets created in the frr-k8s namespace. Signed-off-by: Federico Paolinelli <[email protected]>
fedepaol
force-pushed
the
passwordsecrets
branch
2 times, most recently
from
c89ac75
to
0012e20
Compare
Include Regenerating the all-in-one manifests for secrets rbac Signed-off-by: Federico Paolinelli <[email protected]>
Creating the secrets and configuring the neighbors so the session uses the password. The EBGP-multihop session is left without password so we validate it keeps working. Signed-off-by: Federico Paolinelli <[email protected]>
fedepaol
force-pushed
the
passwordsecrets
branch
from
0012e20
to
ce97ee1
Compare
Handling passwords as secret references.