From be1deb7d4bbab1cfe98ac91f6ba177e7bbcded4c Mon Sep 17 00:00:00 2001 From: Moritz Sanft <58110325+msanft@users.noreply.github.com> Date: Wed, 11 Dec 2024 17:04:13 +0100 Subject: [PATCH] Revert "operator: use GCP REST API for instance templates (#3361)" (#3544) This reverts commit effb086cd33129a3b4046419b4615b43d4380038. --- .../internal/cloud/gcp/client/BUILD.bazel | 2 - .../internal/cloud/gcp/client/api.go | 11 ++- .../internal/cloud/gcp/client/client.go | 11 +-- .../internal/cloud/gcp/client/client_test.go | 27 ++++-- .../internal/cloud/gcp/client/gcpwrappers.go | 21 ++-- .../internal/cloud/gcp/client/scalinggroup.go | 38 +++++--- .../cloud/gcp/client/scalinggroup_test.go | 97 +++++++++---------- 7 files changed, 113 insertions(+), 94 deletions(-) diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/BUILD.bazel b/operators/constellation-node-operator/internal/cloud/gcp/client/BUILD.bazel index 54100de98f..72548055d5 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/BUILD.bazel +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/BUILD.bazel @@ -29,7 +29,6 @@ go_library( "@com_github_spf13_afero//:afero", "@com_google_cloud_go_compute//apiv1", "@com_google_cloud_go_compute//apiv1/computepb", - "@org_golang_google_api//compute/v1:compute", "@org_golang_google_api//googleapi", "@org_golang_google_api//iterator", "@org_golang_google_protobuf//proto", @@ -62,7 +61,6 @@ go_test( "@com_github_stretchr_testify//require", "@com_google_cloud_go_compute//apiv1", "@com_google_cloud_go_compute//apiv1/computepb", - "@org_golang_google_api//compute/v1:compute", "@org_golang_google_api//googleapi", "@org_golang_google_api//iterator", "@org_golang_google_protobuf//proto", diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/api.go b/operators/constellation-node-operator/internal/cloud/gcp/client/api.go index 15c56ece5c..12966da9ea 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/api.go +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/api.go @@ -12,7 +12,6 @@ import ( compute "cloud.google.com/go/compute/apiv1" "cloud.google.com/go/compute/apiv1/computepb" "github.com/googleapis/gax-go/v2" - computeREST "google.golang.org/api/compute/v1" ) type projectAPI interface { @@ -28,9 +27,13 @@ type instanceAPI interface { } type instanceTemplateAPI interface { - Get(projectID, template string) (*computeREST.InstanceTemplate, error) - Delete(projectID, template string) (*computeREST.Operation, error) - Insert(projectID string, template *computeREST.InstanceTemplate) (*computeREST.Operation, error) + Close() error + Get(ctx context.Context, req *computepb.GetInstanceTemplateRequest, + opts ...gax.CallOption) (*computepb.InstanceTemplate, error) + Delete(ctx context.Context, req *computepb.DeleteInstanceTemplateRequest, + opts ...gax.CallOption) (Operation, error) + Insert(ctx context.Context, req *computepb.InsertInstanceTemplateRequest, + opts ...gax.CallOption) (Operation, error) } type instanceGroupManagersAPI interface { diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/client.go b/operators/constellation-node-operator/internal/cloud/gcp/client/client.go index e9dbc3a4e3..aa0a46ae71 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/client.go +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/client.go @@ -14,7 +14,6 @@ import ( compute "cloud.google.com/go/compute/apiv1" "github.com/spf13/afero" - computeREST "google.golang.org/api/compute/v1" ) // Client is a client for the Google Compute Engine. @@ -49,17 +48,12 @@ func New(ctx context.Context, configPath string) (*Client, error) { return nil, err } closers = append(closers, insAPI) - - // TODO(msanft): Go back to protobuf-based API when it supports setting - // a confidential instance type. - // See https://github.com/googleapis/google-cloud-go/issues/10873 for the current status. - restClient, err := computeREST.NewService(ctx) + templAPI, err := compute.NewInstanceTemplatesRESTClient(ctx) if err != nil { _ = closeAll(closers) return nil, err } - templAPI := computeREST.NewInstanceTemplatesService(restClient) - + closers = append(closers, templAPI) groupAPI, err := compute.NewInstanceGroupManagersRESTClient(ctx) if err != nil { _ = closeAll(closers) @@ -87,6 +81,7 @@ func (c *Client) Close() error { closers := []closer{ c.projectAPI, c.instanceAPI, + c.instanceTemplateAPI, c.instanceGroupManagersAPI, c.diskAPI, } diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/client_test.go b/operators/constellation-node-operator/internal/cloud/gcp/client/client_test.go index 58816c5619..e7779453b8 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/client_test.go +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/client_test.go @@ -12,7 +12,6 @@ import ( compute "cloud.google.com/go/compute/apiv1" "cloud.google.com/go/compute/apiv1/computepb" "github.com/googleapis/gax-go/v2" - computeREST "google.golang.org/api/compute/v1" "google.golang.org/api/iterator" "google.golang.org/protobuf/proto" ) @@ -48,7 +47,7 @@ func (a stubInstanceAPI) Get(_ context.Context, _ *computepb.GetInstanceRequest, } type stubInstanceTemplateAPI struct { - template *computeREST.InstanceTemplate + template *computepb.InstanceTemplate getErr error deleteErr error insertErr error @@ -58,16 +57,30 @@ func (a stubInstanceTemplateAPI) Close() error { return nil } -func (a stubInstanceTemplateAPI) Get(_, _ string) (*computeREST.InstanceTemplate, error) { +func (a stubInstanceTemplateAPI) Get(_ context.Context, _ *computepb.GetInstanceTemplateRequest, + _ ...gax.CallOption, +) (*computepb.InstanceTemplate, error) { return a.template, a.getErr } -func (a stubInstanceTemplateAPI) Delete(_, _ string) (*computeREST.Operation, error) { - return &computeREST.Operation{}, a.deleteErr +func (a stubInstanceTemplateAPI) Delete(_ context.Context, _ *computepb.DeleteInstanceTemplateRequest, + _ ...gax.CallOption, +) (Operation, error) { + return &stubOperation{ + &computepb.Operation{ + Name: proto.String("name"), + }, + }, a.deleteErr } -func (a stubInstanceTemplateAPI) Insert(_ string, _ *computeREST.InstanceTemplate) (*computeREST.Operation, error) { - return &computeREST.Operation{}, a.insertErr +func (a stubInstanceTemplateAPI) Insert(_ context.Context, _ *computepb.InsertInstanceTemplateRequest, + _ ...gax.CallOption, +) (Operation, error) { + return &stubOperation{ + &computepb.Operation{ + Name: proto.String("name"), + }, + }, a.insertErr } type stubInstanceGroupManagersAPI struct { diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/gcpwrappers.go b/operators/constellation-node-operator/internal/cloud/gcp/client/gcpwrappers.go index da87f596cd..3d34efba75 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/gcpwrappers.go +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/gcpwrappers.go @@ -12,27 +12,26 @@ import ( compute "cloud.google.com/go/compute/apiv1" "cloud.google.com/go/compute/apiv1/computepb" "github.com/googleapis/gax-go/v2" - computeREST "google.golang.org/api/compute/v1" ) type instanceTemplateClient struct { - *computeREST.InstanceTemplatesService + *compute.InstanceTemplatesClient } func (c *instanceTemplateClient) Close() error { - return nil // no-op + return c.InstanceTemplatesClient.Close() } -func (c *instanceTemplateClient) Get(project, template string) (*computeREST.InstanceTemplate, error) { - return c.InstanceTemplatesService.Get(project, template).Do() -} - -func (c *instanceTemplateClient) Delete(project, template string) (*computeREST.Operation, error) { - return c.InstanceTemplatesService.Delete(project, template).Do() +func (c *instanceTemplateClient) Delete(ctx context.Context, req *computepb.DeleteInstanceTemplateRequest, + opts ...gax.CallOption, +) (Operation, error) { + return c.InstanceTemplatesClient.Delete(ctx, req, opts...) } -func (c *instanceTemplateClient) Insert(projectID string, template *computeREST.InstanceTemplate) (*computeREST.Operation, error) { - return c.InstanceTemplatesService.Insert(projectID, template).Do() +func (c *instanceTemplateClient) Insert(ctx context.Context, req *computepb.InsertInstanceTemplateRequest, + opts ...gax.CallOption, +) (Operation, error) { + return c.InstanceTemplatesClient.Insert(ctx, req, opts...) } type instanceGroupManagersClient struct { diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup.go b/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup.go index 3f10af753f..d5bef4ab7d 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup.go +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup.go @@ -16,7 +16,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/constants" updatev1alpha1 "github.com/edgelesssys/constellation/v2/operators/constellation-node-operator/api/v1alpha1" cspapi "github.com/edgelesssys/constellation/v2/operators/constellation-node-operator/internal/cloud/api" - computeREST "google.golang.org/api/compute/v1" "google.golang.org/api/iterator" ) @@ -50,22 +49,29 @@ func (c *Client) SetScalingGroupImage(ctx context.Context, scalingGroupID, image } // clone template with desired image - if instanceTemplate.Name == "" { + if instanceTemplate.Name == nil { return fmt.Errorf("instance template of scaling group %q has no name", scalingGroupID) } - instanceTemplate.Properties.Disks[0].InitializeParams.SourceImage = imageURI - newTemplateName, err := generateInstanceTemplateName(instanceTemplate.Name) + instanceTemplate.Properties.Disks[0].InitializeParams.SourceImage = &imageURI + newTemplateName, err := generateInstanceTemplateName(*instanceTemplate.Name) if err != nil { return err } - instanceTemplate.Name = newTemplateName - if _, err := c.instanceTemplateAPI.Insert(project, instanceTemplate); err != nil { + instanceTemplate.Name = &newTemplateName + op, err := c.instanceTemplateAPI.Insert(ctx, &computepb.InsertInstanceTemplateRequest{ + Project: project, + InstanceTemplateResource: instanceTemplate, + }) + if err != nil { return fmt.Errorf("cloning instance template: %w", err) } + if err := op.Wait(ctx); err != nil { + return fmt.Errorf("waiting for cloned instance template: %w", err) + } newTemplateURI := joinInstanceTemplateURI(project, newTemplateName) // update instance group manager to use new template - op, err := c.instanceGroupManagersAPI.SetInstanceTemplate(ctx, &computepb.SetInstanceTemplateInstanceGroupManagerRequest{ + op, err = c.instanceGroupManagersAPI.SetInstanceTemplate(ctx, &computepb.SetInstanceTemplateInstanceGroupManagerRequest{ InstanceGroupManager: instanceGroupName, Project: project, Zone: zone, @@ -129,7 +135,10 @@ func (c *Client) ListScalingGroups(ctx context.Context, uid string) ([]cspapi.Sc if len(templateURI) < 1 { continue // invalid template URI } - template, err := c.instanceTemplateAPI.Get(c.projectID, templateURI[len(templateURI)-1]) + template, err := c.instanceTemplateAPI.Get(ctx, &computepb.GetInstanceTemplateRequest{ + Project: c.projectID, + InstanceTemplate: templateURI[len(templateURI)-1], + }) if err != nil { retErr = errors.Join(retErr, fmt.Errorf("getting instance template %q: %w", templateURI[len(templateURI)-1], err)) continue @@ -190,7 +199,7 @@ func (c *Client) ListScalingGroups(ctx context.Context, uid string) ([]cspapi.Sc return results, nil } -func (c *Client) getScalingGroupTemplate(ctx context.Context, scalingGroupID string) (*computeREST.InstanceTemplate, error) { +func (c *Client) getScalingGroupTemplate(ctx context.Context, scalingGroupID string) (*computepb.InstanceTemplate, error) { project, zone, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) if err != nil { return nil, err @@ -210,19 +219,22 @@ func (c *Client) getScalingGroupTemplate(ctx context.Context, scalingGroupID str if err != nil { return nil, fmt.Errorf("splitting instance template name: %w", err) } - instanceTemplate, err := c.instanceTemplateAPI.Get(instanceTemplateProject, instanceTemplateName) + instanceTemplate, err := c.instanceTemplateAPI.Get(ctx, &computepb.GetInstanceTemplateRequest{ + InstanceTemplate: instanceTemplateName, + Project: instanceTemplateProject, + }) if err != nil { return nil, fmt.Errorf("getting instance template %q: %w", instanceTemplateName, err) } return instanceTemplate, nil } -func instanceTemplateSourceImage(instanceTemplate *computeREST.InstanceTemplate) (string, error) { +func instanceTemplateSourceImage(instanceTemplate *computepb.InstanceTemplate) (string, error) { if instanceTemplate.Properties == nil || len(instanceTemplate.Properties.Disks) == 0 || instanceTemplate.Properties.Disks[0].InitializeParams == nil || - instanceTemplate.Properties.Disks[0].InitializeParams.SourceImage == "" { + instanceTemplate.Properties.Disks[0].InitializeParams.SourceImage == nil { return "", errors.New("instance template has no source image") } - return uriNormalize(instanceTemplate.Properties.Disks[0].InitializeParams.SourceImage), nil + return uriNormalize(*instanceTemplate.Properties.Disks[0].InitializeParams.SourceImage), nil } diff --git a/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup_test.go b/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup_test.go index 01629951d2..5fd3b1170e 100644 --- a/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup_test.go +++ b/operators/constellation-node-operator/internal/cloud/gcp/client/scalinggroup_test.go @@ -16,7 +16,6 @@ import ( cspapi "github.com/edgelesssys/constellation/v2/operators/constellation-node-operator/internal/cloud/api" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - computeREST "google.golang.org/api/compute/v1" "google.golang.org/protobuf/proto" ) @@ -24,7 +23,7 @@ func TestGetScalingGroupImage(t *testing.T) { testCases := map[string]struct { scalingGroupID string instanceGroupManagerTemplateID *string - instanceTemplate *computeREST.InstanceTemplate + instanceTemplate *computepb.InstanceTemplate getInstanceGroupManagerErr error getInstanceTemplateErr error wantImage string @@ -33,12 +32,12 @@ func TestGetScalingGroupImage(t *testing.T) { "getting image works": { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Properties: &computeREST.InstanceProperties{ - Disks: []*computeREST.AttachedDisk{ + instanceTemplate: &computepb.InstanceTemplate{ + Properties: &computepb.InstanceProperties{ + Disks: []*computepb.AttachedDisk{ { - InitializeParams: &computeREST.AttachedDiskInitializeParams{ - SourceImage: "https://www.googleapis.com/compute/v1/projects/project/global/images/image", + InitializeParams: &computepb.AttachedDiskInitializeParams{ + SourceImage: proto.String("https://www.googleapis.com/compute/v1/projects/project/global/images/image"), }, }, }, @@ -73,8 +72,8 @@ func TestGetScalingGroupImage(t *testing.T) { "instance template has no disks": { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Properties: &computeREST.InstanceProperties{}, + instanceTemplate: &computepb.InstanceTemplate{ + Properties: &computepb.InstanceProperties{}, }, wantErr: true, }, @@ -113,7 +112,7 @@ func TestSetScalingGroupImage(t *testing.T) { scalingGroupID string imageURI string instanceGroupManagerTemplateID *string - instanceTemplate *computeREST.InstanceTemplate + instanceTemplate *computepb.InstanceTemplate getInstanceGroupManagerErr error getInstanceTemplateErr error setInstanceTemplateErr error @@ -124,13 +123,13 @@ func TestSetScalingGroupImage(t *testing.T) { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Name: "instance-template", - Properties: &computeREST.InstanceProperties{ - Disks: []*computeREST.AttachedDisk{ + instanceTemplate: &computepb.InstanceTemplate{ + Name: proto.String("instance-template"), + Properties: &computepb.InstanceProperties{ + Disks: []*computepb.AttachedDisk{ { - InitializeParams: &computeREST.AttachedDiskInitializeParams{ - SourceImage: "https://www.googleapis.com/compute/v1/projects/project/global/images/image-1", + InitializeParams: &computepb.AttachedDiskInitializeParams{ + SourceImage: proto.String("https://www.googleapis.com/compute/v1/projects/project/global/images/image-1"), }, }, }, @@ -141,13 +140,13 @@ func TestSetScalingGroupImage(t *testing.T) { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Name: "instance-template", - Properties: &computeREST.InstanceProperties{ - Disks: []*computeREST.AttachedDisk{ + instanceTemplate: &computepb.InstanceTemplate{ + Name: proto.String("instance-template"), + Properties: &computepb.InstanceProperties{ + Disks: []*computepb.AttachedDisk{ { - InitializeParams: &computeREST.AttachedDiskInitializeParams{ - SourceImage: "https://www.googleapis.com/compute/v1/projects/project/global/images/image", + InitializeParams: &computepb.AttachedDiskInitializeParams{ + SourceImage: proto.String("https://www.googleapis.com/compute/v1/projects/project/global/images/image"), }, }, }, @@ -183,8 +182,8 @@ func TestSetScalingGroupImage(t *testing.T) { "instance template has no disks": { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Properties: &computeREST.InstanceProperties{}, + instanceTemplate: &computepb.InstanceTemplate{ + Properties: &computepb.InstanceProperties{}, }, wantErr: true, }, @@ -192,12 +191,12 @@ func TestSetScalingGroupImage(t *testing.T) { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Properties: &computeREST.InstanceProperties{ - Disks: []*computeREST.AttachedDisk{ + instanceTemplate: &computepb.InstanceTemplate{ + Properties: &computepb.InstanceProperties{ + Disks: []*computepb.AttachedDisk{ { - InitializeParams: &computeREST.AttachedDiskInitializeParams{ - SourceImage: "https://www.googleapis.com/compute/v1/projects/project/global/images/image-1", + InitializeParams: &computepb.AttachedDiskInitializeParams{ + SourceImage: proto.String("https://www.googleapis.com/compute/v1/projects/project/global/images/image-1"), }, }, }, @@ -209,13 +208,13 @@ func TestSetScalingGroupImage(t *testing.T) { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Name: "instance-template-999999999999999999999", - Properties: &computeREST.InstanceProperties{ - Disks: []*computeREST.AttachedDisk{ + instanceTemplate: &computepb.InstanceTemplate{ + Name: proto.String("instance-template-999999999999999999999"), + Properties: &computepb.InstanceProperties{ + Disks: []*computepb.AttachedDisk{ { - InitializeParams: &computeREST.AttachedDiskInitializeParams{ - SourceImage: "https://www.googleapis.com/compute/v1/projects/project/global/images/image-1", + InitializeParams: &computepb.AttachedDiskInitializeParams{ + SourceImage: proto.String("https://www.googleapis.com/compute/v1/projects/project/global/images/image-1"), }, }, }, @@ -227,13 +226,13 @@ func TestSetScalingGroupImage(t *testing.T) { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Name: "instance-template", - Properties: &computeREST.InstanceProperties{ - Disks: []*computeREST.AttachedDisk{ + instanceTemplate: &computepb.InstanceTemplate{ + Name: proto.String("instance-template"), + Properties: &computepb.InstanceProperties{ + Disks: []*computepb.AttachedDisk{ { - InitializeParams: &computeREST.AttachedDiskInitializeParams{ - SourceImage: "https://www.googleapis.com/compute/v1/projects/project/global/images/image-1", + InitializeParams: &computepb.AttachedDiskInitializeParams{ + SourceImage: proto.String("https://www.googleapis.com/compute/v1/projects/project/global/images/image-1"), }, }, }, @@ -246,13 +245,13 @@ func TestSetScalingGroupImage(t *testing.T) { scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), - instanceTemplate: &computeREST.InstanceTemplate{ - Name: "instance-template", - Properties: &computeREST.InstanceProperties{ - Disks: []*computeREST.AttachedDisk{ + instanceTemplate: &computepb.InstanceTemplate{ + Name: proto.String("instance-template"), + Properties: &computepb.InstanceProperties{ + Disks: []*computepb.AttachedDisk{ { - InitializeParams: &computeREST.AttachedDiskInitializeParams{ - SourceImage: "https://www.googleapis.com/compute/v1/projects/project/global/images/image-1", + InitializeParams: &computepb.AttachedDiskInitializeParams{ + SourceImage: proto.String("https://www.googleapis.com/compute/v1/projects/project/global/images/image-1"), }, }, }, @@ -449,8 +448,8 @@ func TestListScalingGroups(t *testing.T) { }, }, instanceTemplateAPI: &stubInstanceTemplateAPI{ - template: &computeREST.InstanceTemplate{ - Properties: &computeREST.InstanceProperties{ + template: &computepb.InstanceTemplate{ + Properties: &computepb.InstanceProperties{ Labels: tc.templateLabels, }, },