From 80ebfab164955ca438db8deeff2f10632dde810e Mon Sep 17 00:00:00 2001 From: Malte Poll Date: Fri, 5 Aug 2022 12:15:06 +0200 Subject: [PATCH] [node operator] GCP: use canonical references --- .../internal/gcp/client/api.go | 6 ++ .../internal/gcp/client/client.go | 10 +++ .../internal/gcp/client/client_test.go | 15 +++++ .../gcp/client/instancegroupmanagers.go | 13 ++++ .../internal/gcp/client/nodeimage.go | 12 ++++ .../internal/gcp/client/nodeimage_test.go | 4 +- .../internal/gcp/client/project.go | 26 ++++++++ .../internal/gcp/client/project_test.go | 63 +++++++++++++++++++ .../internal/gcp/client/scalinggroup.go | 5 +- 9 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 operators/constellation-node-operator/internal/gcp/client/project.go create mode 100644 operators/constellation-node-operator/internal/gcp/client/project_test.go diff --git a/operators/constellation-node-operator/internal/gcp/client/api.go b/operators/constellation-node-operator/internal/gcp/client/api.go index dfade861e..40690d2ae 100644 --- a/operators/constellation-node-operator/internal/gcp/client/api.go +++ b/operators/constellation-node-operator/internal/gcp/client/api.go @@ -8,6 +8,12 @@ import ( computepb "google.golang.org/genproto/googleapis/cloud/compute/v1" ) +type projectAPI interface { + Close() error + Get(ctx context.Context, req *computepb.GetProjectRequest, + opts ...gax.CallOption) (*computepb.Project, error) +} + type instanceAPI interface { Close() error Get(ctx context.Context, req *computepb.GetInstanceRequest, diff --git a/operators/constellation-node-operator/internal/gcp/client/client.go b/operators/constellation-node-operator/internal/gcp/client/client.go index d2f3f8993..06977c0f6 100644 --- a/operators/constellation-node-operator/internal/gcp/client/client.go +++ b/operators/constellation-node-operator/internal/gcp/client/client.go @@ -13,6 +13,7 @@ import ( // Client is a client for the Google Compute Engine. type Client struct { projectID string + projectAPI instanceAPI instanceTemplateAPI instanceGroupManagersAPI @@ -29,8 +30,15 @@ func New(ctx context.Context, configPath string) (*Client, error) { } var closers []closer + projectAPI, err := compute.NewProjectsRESTClient(ctx) + if err != nil { + return nil, err + } + + closers = append(closers, projectAPI) insAPI, err := compute.NewInstancesRESTClient(ctx) if err != nil { + _ = closeAll(closers) return nil, err } closers = append(closers, insAPI) @@ -53,6 +61,7 @@ func New(ctx context.Context, configPath string) (*Client, error) { } return &Client{ projectID: projectID, + projectAPI: projectAPI, instanceAPI: insAPI, instanceTemplateAPI: &instanceTemplateClient{templAPI}, instanceGroupManagersAPI: &instanceGroupManagersClient{groupAPI}, @@ -64,6 +73,7 @@ func New(ctx context.Context, configPath string) (*Client, error) { // Close closes the client's connection. func (c *Client) Close() error { closers := []closer{ + c.projectAPI, c.instanceAPI, c.instanceTemplateAPI, c.instanceGroupManagersAPI, diff --git a/operators/constellation-node-operator/internal/gcp/client/client_test.go b/operators/constellation-node-operator/internal/gcp/client/client_test.go index 12e4bc998..df7ae16f2 100644 --- a/operators/constellation-node-operator/internal/gcp/client/client_test.go +++ b/operators/constellation-node-operator/internal/gcp/client/client_test.go @@ -10,6 +10,21 @@ import ( "google.golang.org/protobuf/proto" ) +type stubProjectAPI struct { + project *computepb.Project + getErr error +} + +func (a stubProjectAPI) Close() error { + return nil +} + +func (a stubProjectAPI) Get(ctx context.Context, req *computepb.GetProjectRequest, + opts ...gax.CallOption, +) (*computepb.Project, error) { + return a.project, a.getErr +} + type stubInstanceAPI struct { instance *computepb.Instance getErr error diff --git a/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers.go b/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers.go index c1baba59a..10753971a 100644 --- a/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers.go +++ b/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers.go @@ -1,6 +1,7 @@ package client import ( + "context" "fmt" "regexp" ) @@ -11,6 +12,18 @@ var ( workerInstanceGroupNameRegex = regexp.MustCompile(`^(.*)worker(.*)$`) ) +func (c *Client) canonicalInstanceGroupID(ctx context.Context, instanceGroupID string) (string, error) { + project, zone, instanceGroup, err := splitInstanceGroupID(uriNormalize(instanceGroupID)) + if err != nil { + return "", err + } + project, err = c.canonicalProjectID(ctx, project) + if err != nil { + return "", err + } + return fmt.Sprintf("projects/%s/zones/%s/instanceGroupManagers/%s", project, zone, instanceGroup), nil +} + // splitInstanceGroupID splits an instance group ID into core components. func splitInstanceGroupID(instanceGroupID string) (project, zone, instanceGroup string, err error) { matches := instanceGroupIDRegex.FindStringSubmatch(instanceGroupID) diff --git a/operators/constellation-node-operator/internal/gcp/client/nodeimage.go b/operators/constellation-node-operator/internal/gcp/client/nodeimage.go index 25527a4f4..c28f81b3b 100644 --- a/operators/constellation-node-operator/internal/gcp/client/nodeimage.go +++ b/operators/constellation-node-operator/internal/gcp/client/nodeimage.go @@ -14,6 +14,10 @@ func (c *Client) GetNodeImage(ctx context.Context, providerID string) (string, e if err != nil { return "", err } + project, err = c.canonicalProjectID(ctx, project) + if err != nil { + return "", err + } instance, err := c.instanceAPI.Get(ctx, &computepb.GetInstanceRequest{ Instance: instanceName, Project: project, @@ -61,6 +65,10 @@ func (c *Client) GetScalingGroupID(ctx context.Context, providerID string) (stri if scalingGroupID == "" { return "", fmt.Errorf("instance %q has no created-by metadata", instanceName) } + scalingGroupID, err = c.canonicalInstanceGroupID(ctx, scalingGroupID) + if err != nil { + return "", err + } return scalingGroupID, nil } @@ -70,6 +78,10 @@ func (c *Client) CreateNode(ctx context.Context, scalingGroupID string) (nodeNam if err != nil { return "", "", err } + project, err = c.canonicalProjectID(ctx, project) + if err != nil { + return "", "", err + } instanceGroupManager, err := c.instanceGroupManagersAPI.Get(ctx, &computepb.GetInstanceGroupManagerRequest{ InstanceGroupManager: instanceGroupName, Project: project, diff --git a/operators/constellation-node-operator/internal/gcp/client/nodeimage_test.go b/operators/constellation-node-operator/internal/gcp/client/nodeimage_test.go index 1bca6bdd9..e01f62af7 100644 --- a/operators/constellation-node-operator/internal/gcp/client/nodeimage_test.go +++ b/operators/constellation-node-operator/internal/gcp/client/nodeimage_test.go @@ -116,8 +116,8 @@ func TestGetScalingGroupID(t *testing.T) { }{ "scaling group is found": { providerID: "gce://project/zone/instance-name", - createdBy: "projects/project/zones/zone/instanceGroups/instance-group", - wantScalingGroupID: "projects/project/zones/zone/instanceGroups/instance-group", + createdBy: "projects/project/zones/zone/instanceGroupManagers/instance-group", + wantScalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", }, "splitting providerID fails": { providerID: "invalid", diff --git a/operators/constellation-node-operator/internal/gcp/client/project.go b/operators/constellation-node-operator/internal/gcp/client/project.go new file mode 100644 index 000000000..869e39564 --- /dev/null +++ b/operators/constellation-node-operator/internal/gcp/client/project.go @@ -0,0 +1,26 @@ +package client + +import ( + "context" + "errors" + "regexp" + + "google.golang.org/genproto/googleapis/cloud/compute/v1" +) + +var numericProjectIDRegex = regexp.MustCompile(`^\d+$`) + +// canonicalProjectID returns the project id for a given project id or project number. +func (c *Client) canonicalProjectID(ctx context.Context, project string) (string, error) { + if !numericProjectIDRegex.MatchString(project) { + return project, nil + } + computeProject, err := c.projectAPI.Get(ctx, &compute.GetProjectRequest{Project: project}) + if err != nil { + return "", err + } + if computeProject == nil || computeProject.Name == nil { + return "", errors.New("invalid project") + } + return *computeProject.Name, nil +} diff --git a/operators/constellation-node-operator/internal/gcp/client/project_test.go b/operators/constellation-node-operator/internal/gcp/client/project_test.go new file mode 100644 index 000000000..d66d556b3 --- /dev/null +++ b/operators/constellation-node-operator/internal/gcp/client/project_test.go @@ -0,0 +1,63 @@ +package client + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + computepb "google.golang.org/genproto/googleapis/cloud/compute/v1" + "google.golang.org/protobuf/proto" +) + +func TestCanonicalProjectID(t *testing.T) { + testCases := map[string]struct { + projectID string + project *computepb.Project + getProjectErr error + wantProjectID string + wantErr bool + }{ + "already canonical": { + projectID: "project-12345", + wantProjectID: "project-12345", + }, + "numeric project id": { + projectID: "12345", + wantProjectID: "project-12345", + project: &computepb.Project{Name: proto.String("project-12345")}, + }, + "numeric project id with error": { + projectID: "12345", + wantProjectID: "project-12345", + getProjectErr: errors.New("get error"), + wantErr: true, + }, + "numeric project id with nil project": { + projectID: "12345", + wantErr: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + client := Client{ + projectAPI: &stubProjectAPI{ + project: tc.project, + getErr: tc.getProjectErr, + }, + } + gotID, err := client.canonicalProjectID(context.Background(), tc.projectID) + if tc.wantErr { + assert.Error(err) + return + } + require.NoError(err) + assert.Equal(tc.wantProjectID, gotID) + }) + } +} diff --git a/operators/constellation-node-operator/internal/gcp/client/scalinggroup.go b/operators/constellation-node-operator/internal/gcp/client/scalinggroup.go index 04a7a3d9c..001845e08 100644 --- a/operators/constellation-node-operator/internal/gcp/client/scalinggroup.go +++ b/operators/constellation-node-operator/internal/gcp/client/scalinggroup.go @@ -109,7 +109,10 @@ func (c *Client) ListScalingGroups(ctx context.Context, uid string) (controlPlan if instanceGroupManager == nil || instanceGroupManager.Name == nil || instanceGroupManager.SelfLink == nil { continue } - groupID := uriNormalize(*instanceGroupManager.SelfLink) + groupID, err := c.canonicalInstanceGroupID(ctx, *instanceGroupManager.SelfLink) + if err != nil { + return nil, nil, fmt.Errorf("normalizing instance group ID: %w", err) + } if isControlPlaneInstanceGroup(*instanceGroupManager.Name) { controlPlaneGroupIDs = append(controlPlaneGroupIDs, groupID)