m/p/api: UpdateNodeRoles: identify by node IDs
This adds the possibility of identifying nodes through their node IDs
on top of their public keys.
Change-Id: I4028832773acb11faf306144fece923e6c51d20d
Reviewed-on: https://review.monogon.dev/c/monogon/+/849
Tested-by: Jenkins CI
Reviewed-by: Sergiusz Bazanski <serge@monogon.tech>
diff --git a/metropolis/node/core/curator/impl_leader_management.go b/metropolis/node/core/curator/impl_leader_management.go
index e755f03..819456c 100644
--- a/metropolis/node/core/curator/impl_leader_management.go
+++ b/metropolis/node/core/curator/impl_leader_management.go
@@ -283,8 +283,21 @@
// adjusting the affected node's representation within the cluster, can also
// trigger the addition of a new etcd learner node.
func (l *leaderManagement) UpdateNodeRoles(ctx context.Context, req *apb.UpdateNodeRolesRequest) (*apb.UpdateNodeRolesResponse, error) {
- if len(req.Pubkey) != ed25519.PublicKeySize {
- return nil, status.Errorf(codes.InvalidArgument, "pubkey must be %d bytes long", ed25519.PublicKeySize)
+ // Nodes are identifiable by either of their public keys or (string) node IDs.
+ // In case a public key was provided, convert it to a corresponding node ID
+ // here.
+ var id string
+ switch rid := req.Node.(type) {
+ case *apb.UpdateNodeRolesRequest_Pubkey:
+ if len(rid.Pubkey) != ed25519.PublicKeySize {
+ return nil, status.Errorf(codes.InvalidArgument, "pubkey must be %d bytes long", ed25519.PublicKeySize)
+ }
+ // Convert the pubkey into node ID.
+ id = identity.NodeID(rid.Pubkey)
+ case *apb.UpdateNodeRolesRequest_Id:
+ id = rid.Id
+ default:
+ return nil, status.Errorf(codes.InvalidArgument, "exactly one of pubkey or id must be set")
}
// Take l.muNodes before modifying the node.
@@ -292,8 +305,10 @@
defer l.muNodes.Unlock()
// Find the node matching the requested public key.
- id := identity.NodeID(req.Pubkey)
node, err := nodeLoad(ctx, l.leadership, id)
+ if err == errNodeNotFound {
+ return nil, status.Errorf(codes.NotFound, "node %s not found", id)
+ }
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "while loading node %s: %v", id, err)
}
diff --git a/metropolis/node/core/curator/impl_leader_test.go b/metropolis/node/core/curator/impl_leader_test.go
index 1ede1c5..8cf1c3e 100644
--- a/metropolis/node/core/curator/impl_leader_test.go
+++ b/metropolis/node/core/curator/impl_leader_test.go
@@ -1059,22 +1059,30 @@
opt := func(v bool) *bool { return &v }
ue := []*apb.UpdateNodeRolesRequest{
&apb.UpdateNodeRolesRequest{
- Pubkey: tn[0].pubkey,
+ Node: &apb.UpdateNodeRolesRequest_Pubkey{
+ Pubkey: tn[0].pubkey,
+ },
KubernetesWorker: opt(false),
ConsensusMember: opt(false),
},
&apb.UpdateNodeRolesRequest{
- Pubkey: tn[1].pubkey,
+ Node: &apb.UpdateNodeRolesRequest_Pubkey{
+ Pubkey: tn[1].pubkey,
+ },
KubernetesWorker: opt(false),
ConsensusMember: opt(true),
},
&apb.UpdateNodeRolesRequest{
- Pubkey: tn[2].pubkey,
+ Node: &apb.UpdateNodeRolesRequest_Pubkey{
+ Pubkey: tn[2].pubkey,
+ },
KubernetesWorker: opt(true),
ConsensusMember: opt(true),
},
&apb.UpdateNodeRolesRequest{
- Pubkey: tn[2].pubkey,
+ Node: &apb.UpdateNodeRolesRequest_Pubkey{
+ Pubkey: tn[2].pubkey,
+ },
KubernetesWorker: nil,
ConsensusMember: nil,
},
@@ -1100,7 +1108,7 @@
cn := getNodes(t, ctx, mgmt, "")
for i, e := range ue {
for _, n := range cn {
- if bytes.Equal(n.Pubkey, e.Pubkey) {
+ if bytes.Equal(n.Pubkey, e.GetPubkey()) {
if e.KubernetesWorker != nil {
if *e.KubernetesWorker != (n.Roles.KubernetesWorker != nil) {
t.Fatalf("KubernetesWorker role mismatch (node %d/%d).", i+1, len(ue))
@@ -1120,12 +1128,16 @@
// as well.
uf := []*apb.UpdateNodeRolesRequest{
&apb.UpdateNodeRolesRequest{
- Pubkey: tn[0].pubkey,
+ Node: &apb.UpdateNodeRolesRequest_Pubkey{
+ Pubkey: tn[0].pubkey,
+ },
KubernetesWorker: opt(true),
ConsensusMember: opt(false),
},
&apb.UpdateNodeRolesRequest{
- Pubkey: tn[0].pubkey,
+ Node: &apb.UpdateNodeRolesRequest_Pubkey{
+ Pubkey: tn[0].pubkey,
+ },
KubernetesWorker: opt(true),
ConsensusMember: nil,
},
diff --git a/metropolis/proto/api/management.proto b/metropolis/proto/api/management.proto
index d481339..396df83 100644
--- a/metropolis/proto/api/management.proto
+++ b/metropolis/proto/api/management.proto
@@ -158,9 +158,15 @@
// role fields are optional, and no change will result if they're either unset
// or if their value matches existing state.
message UpdateNodeRolesRequest {
- // pubkey is the Ed25519 public key of this node, which can be used to
- // generate the node's ID. This is always set.
- bytes pubkey = 1;
+ // node uniquely identifies the node subject to this request.
+ oneof node {
+ // pubkey is the Ed25519 public key of this node, which can be used to
+ // generate the node's ID.
+ bytes pubkey = 1;
+ // id is the human-readable identifier of the node, based on its public
+ // key.
+ string id = 4;
+ }
// kubernetesWorker adjusts the appropriate role when set. Nodes performing
// this role must also be consensus members.