From 1363d39c86fd8b2bbae6aa03927240c05f456abb Mon Sep 17 00:00:00 2001 From: Bharath B Date: Sat, 17 Feb 2024 13:42:13 +0530 Subject: [PATCH] CFE-857 : Apply user defined tags on created gcp resources --- data/data/gcp/bootstrap/main.tf | 25 +- data/data/gcp/cluster/main.tf | 2 +- data/data/gcp/cluster/master/main.tf | 13 +- data/data/gcp/cluster/master/variables.tf | 9 + data/data/gcp/variables-gcp.tf | 11 +- pkg/asset/cluster/tfvars/tfvars.go | 9 + .../installconfig/gcp/mock/usertags_mock.go | 83 ++++++ pkg/asset/installconfig/gcp/usertags.go | 246 ++++++++++++++++ pkg/asset/installconfig/gcp/usertags_test.go | 274 ++++++++++++++++++ pkg/asset/installconfig/gcp/validation.go | 10 + pkg/asset/machines/gcp/machines.go | 9 + pkg/tfvars/gcp/gcp.go | 3 + pkg/types/gcp/validation/platform.go | 67 +---- pkg/types/gcp/validation/platform_test.go | 181 ------------ 14 files changed, 689 insertions(+), 253 deletions(-) create mode 100644 pkg/asset/installconfig/gcp/mock/usertags_mock.go create mode 100644 pkg/asset/installconfig/gcp/usertags.go create mode 100644 pkg/asset/installconfig/gcp/usertags_test.go diff --git a/data/data/gcp/bootstrap/main.tf b/data/data/gcp/bootstrap/main.tf index 1647d0b1e3e..2975005e9e4 100644 --- a/data/data/gcp/bootstrap/main.tf +++ b/data/data/gcp/bootstrap/main.tf @@ -18,6 +18,18 @@ resource "google_storage_bucket" "ignition" { labels = var.gcp_extra_labels } +resource "google_tags_location_tag_binding" "user_tag_binding_bucket" { + for_each = var.gcp_extra_tags + + parent = format("//storage.googleapis.com/projects/_/buckets/%s", + google_storage_bucket.ignition.name, + ) + tag_value = each.value + location = var.gcp_region + + depends_on = [google_storage_bucket.ignition] +} + resource "google_storage_bucket_object" "ignition" { bucket = google_storage_bucket.ignition.name name = "bootstrap.ign" @@ -88,10 +100,11 @@ resource "google_compute_instance" "bootstrap" { boot_disk { initialize_params { - type = var.gcp_master_root_volume_type - size = var.gcp_master_root_volume_size - image = var.compute_image - labels = var.gcp_extra_labels + type = var.gcp_master_root_volume_type + size = var.gcp_master_root_volume_size + image = var.compute_image + labels = var.gcp_extra_labels + resource_manager_tags = var.gcp_extra_tags } kms_key_self_link = var.gcp_root_volume_kms_key_link } @@ -138,6 +151,10 @@ resource "google_compute_instance" "bootstrap" { labels = var.gcp_extra_labels + params { + resource_manager_tags = var.gcp_extra_tags + } + lifecycle { # In GCP TF apply is run a second time to remove bootstrap node from LB. # If machine_type = n2-standard series, install will error as TF tries to diff --git a/data/data/gcp/cluster/main.tf b/data/data/gcp/cluster/main.tf index 548110d1c22..6a48d65e5d0 100644 --- a/data/data/gcp/cluster/main.tf +++ b/data/data/gcp/cluster/main.tf @@ -35,6 +35,7 @@ module "master" { confidential_compute = var.gcp_master_confidential_compute on_host_maintenance = var.gcp_master_on_host_maintenance gcp_extra_labels = var.gcp_extra_labels + gcp_extra_tags = var.gcp_extra_tags tags = var.gcp_control_plane_tags } @@ -82,4 +83,3 @@ module "dns" { project_id = var.gcp_project_id gcp_extra_labels = var.gcp_extra_labels } - diff --git a/data/data/gcp/cluster/master/main.tf b/data/data/gcp/cluster/master/main.tf index febfd4991aa..7f5992e953a 100644 --- a/data/data/gcp/cluster/master/main.tf +++ b/data/data/gcp/cluster/master/main.tf @@ -47,10 +47,11 @@ resource "google_compute_instance" "master" { boot_disk { initialize_params { - type = var.root_volume_type - size = var.root_volume_size - image = var.image - labels = var.gcp_extra_labels + type = var.root_volume_type + size = var.root_volume_size + image = var.image + labels = var.gcp_extra_labels + resource_manager_tags = var.gcp_extra_tags } kms_key_self_link = var.root_volume_kms_key_link } @@ -97,6 +98,10 @@ resource "google_compute_instance" "master" { scopes = ["https://www.googleapis.com/auth/cloud-platform"] } + params { + resource_manager_tags = var.gcp_extra_tags + } + lifecycle { # In GCP TF apply is run a second time to remove bootstrap node from LB. # If machine_type = n2-standard series, install will error as TF tries to diff --git a/data/data/gcp/cluster/master/variables.tf b/data/data/gcp/cluster/master/variables.tf index fba96d9df30..f82fc68e22d 100644 --- a/data/data/gcp/cluster/master/variables.tf +++ b/data/data/gcp/cluster/master/variables.tf @@ -85,3 +85,12 @@ variable "on_host_maintenance" { description = "The behavior when a maintenance event occurs." default = "" } + +variable "gcp_extra_tags" { + type = map(string) + description = < maxUserTagLimit { + return fmt.Errorf("more than %d user tags is not allowed, configured count: %d", maxUserTagLimit, len(userTags)) + } + + projectTags, err := mgr.GetProjectTags(ctx, project) + if err != nil { + return err + } + + if dupTags := findDuplicateTags(userTags, projectTags); len(dupTags) != 0 { + return fmt.Errorf("found duplicate tags, %v tags already exist on %s project resource", dupTags, project) + } + + processedTags.make(len(userTags)) + nonexistentTags := make([]string, 0) + for _, tag := range userTags { + name := fmt.Sprintf("%s/%s/%s", tag.ParentID, tag.Key, tag.Value) + tagValue, err := mgr.GetNamespacedTagValue(ctx, name) + if err != nil { + // check and return all non-existing tags at once + // for user to fix in one go. + var gErr *apierror.APIError + // google API returns StatusForbidden or StatusNotFound when the tag + // does not exist, since it could be because of permission issues + // or genuinely tag does not exist. + if errors.As(err, &gErr) && gErr.HTTPCode() == http.StatusForbidden { + logrus.Debugf("does not have permission to access %s tag or does not exist: %d", name, gErr.HTTPCode()) + nonexistentTags = append(nonexistentTags, name) + continue + } + return fmt.Errorf("failed to fetch user-defined tag %s(%d): %w", name, gErr.HTTPCode(), err) + } + processedTags.addTag(tagValue.Parent, tagValue.Name) + } + + if len(nonexistentTags) != 0 { + return fmt.Errorf("does not have permission to access %v tag(s) or does not exist", nonexistentTags) + } + + processedTags.markProcessed() + logrus.Debugf("user defined tags processed list: %v, took %s to finish", processedTags.tags, time.Since(start)) + return nil +} + +// findDuplicateTags returns list of duplicate userTags which are already present +// in the parentTags list. +func findDuplicateTags(userTags []gcp.UserTag, parentTags sets.Set[string]) []string { + dupTags := make([]string, 0) + for _, tag := range userTags { + tagNamespacedName := fmt.Sprintf("%s/%s/%s", tag.ParentID, tag.Key, tag.Value) + if parentTags.Has(tagNamespacedName) { + dupTags = append(dupTags, tagNamespacedName) + } + } + return dupTags +} + +// getCloudResourceServiceForTags returns the client required for querying resource manager resources. +func (m *tagManager) getCloudResourceServiceForTags(ctx context.Context) (*tags.Service, error) { + svc, err := tags.NewService(ctx, option.WithCredentials(m.client.GetCredentials())) + if err != nil { + return nil, fmt.Errorf("failed to create cloud resource service: %w", err) + } + return svc, nil +} + +// GetProjectTags returns the list of effective tags attached to the provided project resource. +func (m *tagManager) GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error) { + const ( + // projectParentPathFmt is the format string for parent path of a project resource. + projectParentPathFmt = "//cloudresourcemanager.googleapis.com/projects/%s" + ) + + ctx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + + service, err := m.getCloudResourceServiceForTags(ctx) + if err != nil { + return nil, fmt.Errorf("failed to create cloud resource service: %w", err) + } + + effectiveTags := sets.New[string]() + effectiveTagsService := tags.NewEffectiveTagsService(service) + effectiveTagsRequest := effectiveTagsService.List().Context(ctx).Parent(fmt.Sprintf(projectParentPathFmt, projectID)) + if err := effectiveTagsRequest.Pages(ctx, func(page *tags.ListEffectiveTagsResponse) error { + for _, effectiveTag := range page.EffectiveTags { + effectiveTags.Insert(effectiveTag.NamespacedTagValue) + } + return nil + }); err != nil { + return nil, err + } + + return effectiveTags, nil +} + +// GetNamespacedTagValue returns the Tag Value metadata fetched using the tag's NamespacedName. +func (m *tagManager) GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*tags.TagValue, error) { + ctx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + + service, err := m.getCloudResourceServiceForTags(ctx) + if err != nil { + return nil, fmt.Errorf("failed to create cloud resource service: %w", err) + } + + tagValuesService := tags.NewTagValuesService(service) + + return tagValuesService.GetNamespaced(). + Context(ctx). + Name(tagNamespacedName). + Do() +} diff --git a/pkg/asset/installconfig/gcp/usertags_test.go b/pkg/asset/installconfig/gcp/usertags_test.go new file mode 100644 index 00000000000..9154618417a --- /dev/null +++ b/pkg/asset/installconfig/gcp/usertags_test.go @@ -0,0 +1,274 @@ +package gcp + +import ( + "context" + "fmt" + "net/http" + "reflect" + "sync" + "testing" + + "github.com/golang/mock/gomock" + "github.com/googleapis/gax-go/v2/apierror" + googleoauth "golang.org/x/oauth2/google" + "google.golang.org/api/cloudresourcemanager/v3" + "google.golang.org/api/googleapi" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/openshift/installer/pkg/asset/installconfig/gcp/mock" + "github.com/openshift/installer/pkg/types/gcp" +) + +var ( + testTags = []gcp.UserTag{ + {ParentID: "openshift", Key: "key1", Value: "value1"}, {ParentID: "openshift", Key: "key2", Value: "value2"}, + {ParentID: "openshift", Key: "key3", Value: "value3"}, {ParentID: "openshift", Key: "key4", Value: "value4"}, + {ParentID: "openshift", Key: "key5", Value: "value5"}, {ParentID: "openshift", Key: "key6", Value: "value6"}, + {ParentID: "openshift", Key: "key7", Value: "value7"}, {ParentID: "openshift", Key: "key8", Value: "value8"}, + {ParentID: "openshift", Key: "key9", Value: "value9"}, {ParentID: "openshift", Key: "key10", Value: "value10"}, + {ParentID: "openshift", Key: "key11", Value: "value11"}, {ParentID: "openshift", Key: "key12", Value: "value12"}, + {ParentID: "openshift", Key: "key13", Value: "value13"}, {ParentID: "openshift", Key: "key14", Value: "value14"}, + {ParentID: "openshift", Key: "key15", Value: "value15"}, {ParentID: "openshift", Key: "key16", Value: "value16"}, + {ParentID: "openshift", Key: "key17", Value: "value17"}, {ParentID: "openshift", Key: "key18", Value: "value18"}, + {ParentID: "openshift", Key: "key19", Value: "value19"}, {ParentID: "openshift", Key: "key20", Value: "value20"}, + {ParentID: "openshift", Key: "key21", Value: "value21"}, {ParentID: "openshift", Key: "key22", Value: "value22"}, + {ParentID: "openshift", Key: "key23", Value: "value23"}, {ParentID: "openshift", Key: "key24", Value: "value24"}, + {ParentID: "openshift", Key: "key25", Value: "value25"}, {ParentID: "openshift", Key: "key26", Value: "value26"}, + {ParentID: "openshift", Key: "key27", Value: "value27"}, {ParentID: "openshift", Key: "key28", Value: "value28"}, + {ParentID: "openshift", Key: "key29", Value: "value29"}, {ParentID: "openshift", Key: "key30", Value: "value30"}, + {ParentID: "openshift", Key: "key31", Value: "value31"}, {ParentID: "openshift", Key: "key32", Value: "value32"}, + {ParentID: "openshift", Key: "key33", Value: "value33"}, {ParentID: "openshift", Key: "key34", Value: "value34"}, + {ParentID: "openshift", Key: "key35", Value: "value35"}, {ParentID: "openshift", Key: "key36", Value: "value36"}, + {ParentID: "openshift", Key: "key37", Value: "value37"}, {ParentID: "openshift", Key: "key38", Value: "value38"}, + {ParentID: "openshift", Key: "key39", Value: "value39"}, {ParentID: "openshift", Key: "key40", Value: "value40"}, + {ParentID: "openshift", Key: "key41", Value: "value41"}, {ParentID: "openshift", Key: "key42", Value: "value42"}, + {ParentID: "openshift", Key: "key43", Value: "value43"}, {ParentID: "openshift", Key: "key44", Value: "value44"}, + {ParentID: "openshift", Key: "key45", Value: "value45"}, {ParentID: "openshift", Key: "key46", Value: "value46"}, + {ParentID: "openshift", Key: "key47", Value: "value47"}, {ParentID: "openshift", Key: "key48", Value: "value48"}, + {ParentID: "openshift", Key: "key49", Value: "value49"}, {ParentID: "openshift", Key: "key50", Value: "value50"}, + {ParentID: "openshift", Key: "key51", Value: "value51"}, {ParentID: "openshift", Key: "key52", Value: "value52"}, + } + + testProcessedTags = map[string]struct { + key string + value string + }{ + "openshift/key1/value1": {key: "tagKeys/0005159847", value: "tagValues/0030662057"}, + "openshift/key2/value2": {key: "tagKeys/0018550390", value: "tagValues/0073384808"}, + "openshift/key3/value3": {key: "tagKeys/0051315427", value: "tagValues/0051662048"}, + "openshift/key4/value4": {key: "tagKeys/0081126737", value: "tagValues/0012526301"}, + "openshift/key5/value5": {key: "tagKeys/0009228350", value: "tagValues/0043054391"}, + "openshift/key6/value6": {key: "tagKeys/0051626174", value: "tagValues/0027708252"}, + "openshift/key7/value7": {key: "tagKeys/0030414034", value: "tagValues/0020489890"}, + "openshift/key8/value8": {key: "tagKeys/0050469265", value: "tagValues/0002904904"}, + "openshift/key9/value9": {key: "tagKeys/0076293608", value: "tagValues/0055629312"}, + "openshift/key10/value10": {key: "tagKeys/0023033386", value: "tagValues/0088842980"}, + "openshift/key11/value11": {key: "tagKeys/0069899625", value: "tagValues/0047417661"}, + "openshift/key12/value12": {key: "tagKeys/0034605069", value: "tagValues/0050384905"}, + "openshift/key13/value13": {key: "tagKeys/0028357547", value: "tagValues/0052268968"}, + "openshift/key14/value14": {key: "tagKeys/0099944474", value: "tagValues/0059052883"}, + "openshift/key15/value15": {key: "tagKeys/0050205103", value: "tagValues/0036307885"}, + "openshift/key16/value16": {key: "tagKeys/0079771629", value: "tagValues/0065673174"}, + "openshift/key17/value17": {key: "tagKeys/0060225722", value: "tagValues/0081145498"}, + "openshift/key18/value18": {key: "tagKeys/0016496476", value: "tagValues/0046494994"}, + "openshift/key19/value19": {key: "tagKeys/0093247819", value: "tagValues/0041540373"}, + "openshift/key20/value20": {key: "tagKeys/0080859513", value: "tagValues/0016693395"}, + "openshift/key21/value21": {key: "tagKeys/0018537779", value: "tagValues/0003454649"}, + "openshift/key22/value22": {key: "tagKeys/0071724280", value: "tagValues/0047292544"}, + "openshift/key23/value23": {key: "tagKeys/0045095645", value: "tagValues/0089378558"}, + "openshift/key24/value24": {key: "tagKeys/0044575217", value: "tagValues/0022754275"}, + "openshift/key25/value25": {key: "tagKeys/0056084774", value: "tagValues/0040808197"}, + "openshift/key26/value26": {key: "tagKeys/0086508506", value: "tagValues/0091979350"}, + "openshift/key27/value27": {key: "tagKeys/0085330359", value: "tagValues/0051833259"}, + "openshift/key28/value28": {key: "tagKeys/0094744916", value: "tagValues/0011642000"}, + "openshift/key29/value29": {key: "tagKeys/0014270555", value: "tagValues/0072404680"}, + "openshift/key30/value30": {key: "tagKeys/0079085850", value: "tagValues/0007793185"}, + "openshift/key31/value31": {key: "tagKeys/0031484153", value: "tagValues/0050294705"}, + "openshift/key32/value32": {key: "tagKeys/0045311563", value: "tagValues/0029329808"}, + "openshift/key33/value33": {key: "tagKeys/0080836115", value: "tagValues/0003514535"}, + "openshift/key34/value34": {key: "tagKeys/0072216154", value: "tagValues/0060486146"}, + "openshift/key35/value35": {key: "tagKeys/0025032284", value: "tagValues/0038979234"}, + "openshift/key36/value36": {key: "tagKeys/0057998529", value: "tagValues/0067716498"}, + "openshift/key37/value37": {key: "tagKeys/0086808493", value: "tagValues/0060060909"}, + "openshift/key38/value38": {key: "tagKeys/0029402635", value: "tagValues/0060648494"}, + "openshift/key39/value39": {key: "tagKeys/0034805062", value: "tagValues/0066064364"}, + "openshift/key40/value40": {key: "tagKeys/0052208445", value: "tagValues/0098376015"}, + "openshift/key41/value41": {key: "tagKeys/0078738427", value: "tagValues/0046807958"}, + "openshift/key42/value42": {key: "tagKeys/0075526710", value: "tagValues/0092960786"}, + "openshift/key43/value43": {key: "tagKeys/0071238172", value: "tagValues/0014715775"}, + "openshift/key44/value44": {key: "tagKeys/0098580341", value: "tagValues/0052744277"}, + "openshift/key45/value45": {key: "tagKeys/0046775969", value: "tagValues/0095864916"}, + "openshift/key46/value46": {key: "tagKeys/0041543652", value: "tagValues/0054302656"}, + "openshift/key47/value47": {key: "tagKeys/0043236402", value: "tagValues/0087723355"}, + "openshift/key48/value48": {key: "tagKeys/0098289690", value: "tagValues/0018491792"}, + "openshift/key49/value49": {key: "tagKeys/0022125412", value: "tagValues/0063407483"}, + "openshift/key50/value50": {key: "tagKeys/0004322107", value: "tagValues/0083960428"}, + "openshift/key51/value51": {key: "tagKeys/0070391549", value: "tagValues/0027514043"}, + "openshift/key51/value52": {key: "tagKeys/0070391542", value: "tagValues/0227514043"}, + } +) + +func generateExpectedProcessedTags(tags []gcp.UserTag) map[string]string { + retTags := make(map[string]string, len(tags)) + for _, tag := range tags { + t := testProcessedTags[fmt.Sprintf("%s/%s/%s", tag.ParentID, tag.Key, tag.Value)] + retTags[t.key] = t.value + } + return retTags +} + +func getTestGetNamespacedTagValueForbiddenError(tagValueNamespacedName string) error { + apiErr, _ := apierror.FromError(fmt.Errorf("%w", &googleapi.Error{ + Code: http.StatusForbidden, + Message: fmt.Sprintf("Permission denied on resource '%s' (or it may not exist).", tagValueNamespacedName), + })) + return apiErr +} + +func getTestGetNamespacedTagValueInternalError(tagValueNamespacedName string) error { + apiErr, _ := apierror.FromError(fmt.Errorf("%w", &googleapi.Error{ + Code: http.StatusInternalServerError, + Message: fmt.Sprintf("Internal error while fetching '%s'", tagValueNamespacedName), + })) + return apiErr +} + +func getTestNamespacedTagValueResp(tagValueNamespacedName string) *cloudresourcemanager.TagValue { + tag := testProcessedTags[tagValueNamespacedName] + return &cloudresourcemanager.TagValue{ + Name: tag.value, + Parent: tag.key, + NamespacedName: tagValueNamespacedName, + } +} + +func resetProcessedTags() { + processedTags.Lock() + defer processedTags.Unlock() + + processedTags.processed = false + processedTags.tags = map[string]string{} +} + +func TestGetUserTags(t *testing.T) { + testProject := "test-project" + // serializer is for serializing the access to processedTags, since + // each scenario expects different set of tags. + serializer := &sync.Mutex{} + + testCases := []struct { + name string + projectID string + userTags []gcp.UserTag + processedTags map[string]string + expectedError string + }{ + { + name: "user tags is empty", + userTags: []gcp.UserTag{}, + processedTags: map[string]string{}, + }, + { + name: "more than max allowed tags configured", + userTags: testTags, + processedTags: nil, + expectedError: "more than 50 user tags is not allowed, configured count: 52", + }, + { + name: "user tags processing passed", + userTags: testTags[2:50], + processedTags: generateExpectedProcessedTags(testTags[2:50]), + }, + { + name: "fetching project tags fails", + projectID: "fail-project", + userTags: testTags[:5], + processedTags: nil, + expectedError: "failed to fetch tags attached to fail-project: The caller does not have permission", + }, + { + name: "found duplicate tags", + projectID: "dup-project", + userTags: testTags[5:10], + processedTags: nil, + expectedError: `found duplicate tags, [openshift/key6/value6 openshift/key7/value7 openshift/key8/value8 openshift/key9/value9 openshift/key10/value10] tags already exist on dup-project project resource`, + }, + { + name: "non-existent tags configured", + userTags: testTags[40:51], + processedTags: nil, + expectedError: `does not have permission to access [openshift/key51/value51] tag(s) or does not exist`, + }, + { + name: "failed to fetch tag", + userTags: testTags[40:], + processedTags: nil, + expectedError: `failed to fetch user-defined tag openshift/key52/value52(500): googleapi: Error 500: Internal error while fetching 'openshift/key52/value52'`, + }, + } + + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + gcpClient := mock.NewMockAPI(mockCtrl) + tagMgr := mock.NewMockTagManager(mockCtrl) + + // Return fake credentials. + gcpClient.EXPECT().GetCredentials().Return(&googleoauth.Credentials{JSON: []byte(fakeCreds)}).AnyTimes() + + // Return ["openshift/key1/value1", "openshift/key2/value2"] tags for "test-project" project. + tagMgr.EXPECT().GetProjectTags(gomock.Any(), testProject).Return(sets.Insert(sets.New[string](), "openshift/key1/value1", "openshift/key2/value2"), nil).AnyTimes() + + // Return error for "fail-project" project. + tagMgr.EXPECT().GetProjectTags(gomock.Any(), "fail-project"). + Return( + nil, + fmt.Errorf("failed to fetch tags attached to fail-project: The caller does not have permission")). + AnyTimes() + + // Return ["openshift/key6/value6", "openshift/key7/value7", "openshift/key8/value8", + // "openshift/key9/value9", "openshift/key10/value10"] tags for "dup-project" project. + tagMgr.EXPECT().GetProjectTags(gomock.Any(), "dup-project"). + Return( + sets.Insert(sets.New[string](), + "openshift/key6/value6", + "openshift/key7/value7", + "openshift/key8/value8", + "openshift/key9/value9", + "openshift/key10/value10", + ), nil).AnyTimes() + + // Return Forbidden error "openshift/key51/value51" tag. + tagMgr.EXPECT().GetNamespacedTagValue(gomock.Any(), "openshift/key51/value51").Return(nil, getTestGetNamespacedTagValueForbiddenError("openshift/key51/value51")).AnyTimes() + + // Return Internal error "openshift/key52/value52" tag. + tagMgr.EXPECT().GetNamespacedTagValue(gomock.Any(), "openshift/key52/value52").Return(nil, getTestGetNamespacedTagValueInternalError("openshift/key52/value52")).AnyTimes() + + // Return details for the requested tag. + tagMgr.EXPECT().GetNamespacedTagValue(gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error) { + return getTestNamespacedTagValueResp(tagNamespacedName), nil + }).AnyTimes() + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + serializer.Lock() + defer serializer.Unlock() + resetProcessedTags() + + projectID := testProject + if tc.projectID != "" { + projectID = tc.projectID + } + tags, err := GetUserTags(context.Background(), tagMgr, projectID, tc.userTags) + + if (err != nil || tc.expectedError != "") && err.Error() != tc.expectedError { + t.Errorf("GetUserTags(): error: got: %v, want: %v", err, tc.expectedError) + } + + if !reflect.DeepEqual(tags, tc.processedTags) { + t.Errorf("GetUserTags(): tags: got: %v, want: %v", tags, tc.processedTags) + } + }) + } +} diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index f0cba7c6088..41b12eb30af 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -66,6 +66,10 @@ func Validate(client API, ic *types.InstallConfig) error { allErrs = append(allErrs, validateServiceAccountPresent(client, ic)...) allErrs = append(allErrs, validateMarketplaceImages(client, ic)...) + if err := validateUserTags(client, ic.Platform.GCP.ProjectID, ic.Platform.GCP.UserTags); err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("platform").Child("gcp").Child("userTags"), ic.Platform.GCP.UserTags, err.Error())) + } + return allErrs.ToAggregate() } @@ -633,3 +637,9 @@ func checkArchitecture(imageArch string, icArch types.Architecture, role string) } return "" } + +// validateUserTags check for existence and accessibility of user-defined tags and persists +// validated tags in-memory. +func validateUserTags(client API, projectID string, userTags []gcp.UserTag) error { + return validateAndPersistUserTags(context.Background(), NewTagManager(client), projectID, userTags) +} diff --git a/pkg/asset/machines/gcp/machines.go b/pkg/asset/machines/gcp/machines.go index a9dfb760fc9..5dbef450dc2 100644 --- a/pkg/asset/machines/gcp/machines.go +++ b/pkg/asset/machines/gcp/machines.go @@ -195,6 +195,14 @@ func provider(clusterID string, platform *gcp.Platform, mpool *gcp.MachinePool, for _, label := range platform.UserLabels { labels[label.Key] = label.Value } + tags := make([]machineapi.ResourceManagerTag, len(platform.UserTags)) + for i, tag := range platform.UserTags { + tags[i] = machineapi.ResourceManagerTag{ + ParentID: tag.ParentID, + Key: tag.Key, + Value: tag.Value, + } + } return &machineapi.GCPMachineProviderSpec{ TypeMeta: metav1.TypeMeta{ APIVersion: "machine.openshift.io/v1beta1", @@ -229,6 +237,7 @@ func provider(clusterID string, platform *gcp.Platform, mpool *gcp.MachinePool, ConfidentialCompute: machineapi.ConfidentialComputePolicy(mpool.ConfidentialCompute), OnHostMaintenance: machineapi.GCPHostMaintenanceType(mpool.OnHostMaintenance), Labels: labels, + ResourceManagerTags: tags, }, nil } diff --git a/pkg/tfvars/gcp/gcp.go b/pkg/tfvars/gcp/gcp.go index 5050ad0a036..6978455b7fb 100644 --- a/pkg/tfvars/gcp/gcp.go +++ b/pkg/tfvars/gcp/gcp.go @@ -51,6 +51,7 @@ type config struct { EnableConfidentialCompute string `json:"gcp_master_confidential_compute,omitempty"` ExtraLabels map[string]string `json:"gcp_extra_labels,omitempty"` UserProvisionedDNS bool `json:"gcp_user_provisioned_dns,omitempty"` + ExtraTags map[string]string `json:"gcp_extra_tags,omitempty"` } // TFVarsSources contains the parameters to be converted into Terraform variables @@ -65,6 +66,7 @@ type TFVarsSources struct { PreexistingNetwork bool InfrastructureName string UserProvisionedDNS bool + UserTags map[string]string } // TFVars generates gcp-specific Terraform variables launching the cluster. @@ -106,6 +108,7 @@ func TFVars(sources TFVarsSources) ([]byte, error) { OnHostMaintenance: string(masterConfig.OnHostMaintenance), ExtraLabels: labels, UserProvisionedDNS: sources.UserProvisionedDNS, + ExtraTags: sources.UserTags, } if masterConfig.Disks[0].EncryptionKey != nil { diff --git a/pkg/types/gcp/validation/platform.go b/pkg/types/gcp/validation/platform.go index 12f02f75b50..8d96d0e267f 100644 --- a/pkg/types/gcp/validation/platform.go +++ b/pkg/types/gcp/validation/platform.go @@ -74,24 +74,13 @@ var ( // userLabelKeyPrefixRegex is for verifying that the label key does not contain restricted prefixes. userLabelKeyPrefixRegex = regexp.MustCompile(`^(?i)(kubernetes\-io|openshift\-io)`) - - // userTagKeyRegex is for verifying that the tag key contains only allowed characters. - userTagKeyRegex = regexp.MustCompile(`^[a-zA-Z0-9]([0-9A-Za-z_.-]{0,61}[a-zA-Z0-9])?$`) - - // userTagValueRegex is for verifying that the tag value contains only allowed characters. - userTagValueRegex = regexp.MustCompile(`^[a-zA-Z0-9]([0-9A-Za-z_.@%=+:,*#&()\[\]{}\-\s]{0,61}[a-zA-Z0-9])?$`) - - // userTagParentIDRegex is for verifying that the tag parentID contains only allowed characters. - userTagParentIDRegex = regexp.MustCompile(`(^[1-9][0-9]{0,31}$)|(^[a-z][a-z0-9-]{4,28}[a-z0-9]$)`) ) -// maxUserLabelLimit is the maximum userLabels that can be configured as defined in openshift/api. -// https://github.com/openshift/api/commit/ae73a19d05c35068af16c9aeff375d0b7c936a8a#diff-07b264a49084976b670fb699badaca1795027d6ea732a99226a5388104f6174fR592-R602 -const maxUserLabelLimit = 32 - -// maxUserTagLimit is the maximum userTags that can be configured as defined in openshift/api. -// https://github.com/openshift/api/commit/ae73a19d05c35068af16c9aeff375d0b7c936a8a#diff-07b264a49084976b670fb699badaca1795027d6ea732a99226a5388104f6174fR604-R613 -const maxUserTagLimit = 50 +const ( + // maxUserLabelLimit is the maximum userLabels that can be configured as defined in openshift/api. + // https://github.com/openshift/api/commit/ae73a19d05c35068af16c9aeff375d0b7c936a8a#diff-07b264a49084976b670fb699badaca1795027d6ea732a99226a5388104f6174fR592-R602 + maxUserLabelLimit = 32 +) // ValidatePlatform checks that the specified platform is valid. func ValidatePlatform(p *gcp.Platform, fldPath *field.Path, ic *types.InstallConfig) field.ErrorList { @@ -128,9 +117,6 @@ func ValidatePlatform(p *gcp.Platform, fldPath *field.Path, ic *types.InstallCon // check if configured userLabels are valid. allErrs = append(allErrs, validateUserLabels(p.UserLabels, fldPath.Child("userLabels"))...) - // check if configured userTags are valid. - allErrs = append(allErrs, validateUserTags(p.UserTags, fldPath.Child("userTags"))...) - return allErrs } @@ -173,46 +159,3 @@ func validateLabel(key, value string) error { } return nil } - -// validateUserTags verifies if configured number of UserTags is not more than -// allowed limit and the tag keys and values are valid. -func validateUserTags(tags []gcp.UserTag, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - if len(tags) == 0 { - return allErrs - } - - if len(tags) > maxUserTagLimit { - allErrs = append(allErrs, field.TooMany(fldPath, len(tags), maxUserTagLimit)) - } - - for _, tag := range tags { - if err := validateTag(tag.ParentID, tag.Key, tag.Value); err != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Key(tag.Key), tag.Value, err.Error())) - } - } - - return allErrs -} - -// validateTag checks the following to ensure that the tag configured is acceptable. Though -// the criteria is for tag resources to pre-exist, tags will be validated to catch the -// error much earlier. -// - The key and value contain only allowed characters. -// - The key and value is not empty and can have at most 63 characters. -// - The ParentID can be either OrganizationID or ProjectID. -// - OrganizationID must consist of decimal numbers, and cannot have leading zeroes. -// - ProjectID must be 6 to 30 characters in length, can only contain lowercase letters, numbers, -// and hyphens, and must start with a letter, and cannot end with a hyphen. -func validateTag(parentID, key, value string) error { - if !userTagParentIDRegex.MatchString(parentID) { - return fmt.Errorf("tag parentID is invalid or contains invalid characters. ParentID can have a maximum of 32 characters and cannot be empty. ParentID can be either OrganizationID or ProjectID. OrganizationID must consist of decimal numbers, and cannot have leading zeroes and ProjectID must be 6 to 30 characters in length, can only contain lowercase letters, numbers, and hyphens, and must start with a letter, and cannot end with a hyphen") - } - if !userTagKeyRegex.MatchString(key) { - return fmt.Errorf("tag key is invalid or contains invalid characters. Tag key can have a maximum of 63 characters and cannot be empty. Tag key must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `._-`") - } - if !userTagValueRegex.MatchString(value) { - return fmt.Errorf("tag value is invalid or contains invalid characters. Tag value can have a maximum of 63 characters and cannot be empty. Tag value must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `_-.@%%=+:,*#&(){}[]` and spaces") - } - return nil -} diff --git a/pkg/types/gcp/validation/platform_test.go b/pkg/types/gcp/validation/platform_test.go index a9ac80ec263..88fde768b05 100644 --- a/pkg/types/gcp/validation/platform_test.go +++ b/pkg/types/gcp/validation/platform_test.go @@ -293,184 +293,3 @@ func TestValidateUserLabels(t *testing.T) { }) } } - -func TestValidateUserTags(t *testing.T) { - fieldPath := "spec.platform.gcp.userTags" - cases := []struct { - name string - userTags []gcp.UserTag - expectedErr string - }{ - { - name: "userTags not configured", - userTags: []gcp.UserTag{}, - expectedErr: "[]", - }, - { - name: "userTags configured", - userTags: []gcp.UserTag{ - {ParentID: "1234567890", Key: "key_2", Value: "value_2"}, - {ParentID: "test-project-123", Key: "key.gcp", Value: "value.3"}, - {ParentID: "1234567890", Key: "keY", Value: "value"}, - {ParentID: "test-project-123", Key: "thisisalongkeywithinlimitof63_characters-whichisallowedfortags", Value: "value"}, - {ParentID: "1234567890", Key: "KEY4", Value: "hisisavaluewithin-63characters_{[(.@%=+: ,*#&)]}forgcptagvalue"}, - {ParentID: "test-project-123", Key: "key1", Value: "value1"}, - }, - expectedErr: "[]", - }, - { - name: "userTags configured is more than max limit", - userTags: []gcp.UserTag{ - {ParentID: "1234567890", Key: "key29", Value: "value29"}, - {ParentID: "test-project-123", Key: "key33", Value: "value33"}, - {ParentID: "1234567890", Key: "key39", Value: "value39"}, - {ParentID: "test-project-123", Key: "key43", Value: "value43"}, - {ParentID: "1234567890", Key: "key5", Value: "value5"}, - {ParentID: "test-project-123", Key: "key6", Value: "value6"}, - {ParentID: "1234567890", Key: "key14", Value: "value14"}, - {ParentID: "test-project-123", Key: "key25", Value: "value25"}, - {ParentID: "1234567890", Key: "key20", Value: "value20"}, - {ParentID: "test-project-123", Key: "key24", Value: "value24"}, - {ParentID: "1234567890", Key: "key40", Value: "value40"}, - {ParentID: "test-project-123", Key: "key46", Value: "value46"}, - {ParentID: "1234567890", Key: "key1", Value: "value1"}, - {ParentID: "test-project-123", Key: "key2", Value: "value2"}, - {ParentID: "1234567890", Key: "key4", Value: "value4"}, - {ParentID: "test-project-123", Key: "key10", Value: "value10"}, - {ParentID: "1234567890", Key: "key51", Value: "value51"}, - {ParentID: "test-project-123", Key: "key8", Value: "value8"}, - {ParentID: "1234567890", Key: "key13", Value: "value13"}, - {ParentID: "test-project-123", Key: "key44", Value: "value44"}, - {ParentID: "1234567890", Key: "key48", Value: "value48"}, - {ParentID: "test-project-123", Key: "key9", Value: "value9"}, - {ParentID: "1234567890", Key: "key17", Value: "value17"}, - {ParentID: "test-project-123", Key: "key18", Value: "value18"}, - {ParentID: "1234567890", Key: "key30", Value: "value30"}, - {ParentID: "test-project-123", Key: "key36", Value: "value36"}, - {ParentID: "1234567890", Key: "key49", Value: "value49"}, - {ParentID: "test-project-123", Key: "key7", Value: "value7"}, - {ParentID: "1234567890", Key: "key15", Value: "value15"}, - {ParentID: "test-project-123", Key: "key22", Value: "value22"}, - {ParentID: "1234567890", Key: "key34", Value: "value34"}, - {ParentID: "test-project-123", Key: "key37", Value: "value37"}, - {ParentID: "1234567890", Key: "key38", Value: "value38"}, - {ParentID: "test-project-123", Key: "key47", Value: "value47"}, - {ParentID: "1234567890", Key: "key12", Value: "value12"}, - {ParentID: "test-project-123", Key: "key16", Value: "value16"}, - {ParentID: "1234567890", Key: "key23", Value: "value23"}, - {ParentID: "test-project-123", Key: "key28", Value: "value28"}, - {ParentID: "1234567890", Key: "key50", Value: "value50"}, - {ParentID: "test-project-123", Key: "key21", Value: "value21"}, - {ParentID: "1234567890", Key: "key26", Value: "value26"}, - {ParentID: "test-project-123", Key: "key35", Value: "value35"}, - {ParentID: "1234567890", Key: "key42", Value: "value42"}, - {ParentID: "test-project-123", Key: "key31", Value: "value31"}, - {ParentID: "1234567890", Key: "key32", Value: "value32"}, - {ParentID: "test-project-123", Key: "key41", Value: "value41"}, - {ParentID: "1234567890", Key: "key45", Value: "value45"}, - {ParentID: "test-project-123", Key: "key3", Value: "value3"}, - {ParentID: "1234567890", Key: "key11", Value: "value11"}, - {ParentID: "test-project-123", Key: "key19", Value: "value19"}, - {ParentID: "1234567890", Key: "key27", Value: "value27"}, - }, - expectedErr: "[spec.platform.gcp.userTags: Too many: 51: must have at most 50 items]", - }, - { - name: "userTags contains key starting with a special character", - userTags: []gcp.UserTag{{ParentID: "1234567890", Key: "_key", Value: "1value"}}, - expectedErr: "[spec.platform.gcp.userTags[_key]: Invalid value: \"1value\": tag key is invalid or contains invalid characters. Tag key can have a maximum of 63 characters and cannot be empty. Tag key must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `._-`]", - }, - { - name: "userTags contains key ending with a special character", - userTags: []gcp.UserTag{{ParentID: "1234567890", Key: "key@", Value: "1value"}}, - expectedErr: "[spec.platform.gcp.userTags[key@]: Invalid value: \"1value\": tag key is invalid or contains invalid characters. Tag key can have a maximum of 63 characters and cannot be empty. Tag key must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `._-`]", - }, - { - name: "userTags contains empty key", - userTags: []gcp.UserTag{{ParentID: "1234567890", Key: "", Value: "value"}}, - expectedErr: "[spec.platform.gcp.userTags[]: Invalid value: \"value\": tag key is invalid or contains invalid characters. Tag key can have a maximum of 63 characters and cannot be empty. Tag key must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `._-`]", - }, - { - name: "userTags contains key length greater than 63", - userTags: []gcp.UserTag{ - { - ParentID: "1234567890", - Key: "thisisalongkeyforlimitof63_characters-whichisnotallowedfortagkey", - Value: "value", - }, - }, - expectedErr: "[spec.platform.gcp.userTags[thisisalongkeyforlimitof63_characters-whichisnotallowedfortagkey]: Invalid value: \"value\": tag key is invalid or contains invalid characters. Tag key can have a maximum of 63 characters and cannot be empty. Tag key must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `._-`]", - }, - { - name: "userTags contains key with invalid character", - userTags: []gcp.UserTag{{ParentID: "1234567890", Key: "key/test", Value: "value"}}, - expectedErr: "[spec.platform.gcp.userTags[key/test]: Invalid value: \"value\": tag key is invalid or contains invalid characters. Tag key can have a maximum of 63 characters and cannot be empty. Tag key must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `._-`]", - }, - { - name: "userTags contains value length greater than 63", - userTags: []gcp.UserTag{ - { - ParentID: "1234567890", - Key: "key", - Value: "hisisavaluewith-63characters_{[(.@%=+: ,*#&)]}allowedforgcptagvalue", - }, - }, - expectedErr: "[spec.platform.gcp.userTags[key]: Invalid value: \"hisisavaluewith-63characters_{[(.@%=+: ,*#&)]}allowedforgcptagvalue\": tag value is invalid or contains invalid characters. Tag value can have a maximum of 63 characters and cannot be empty. Tag value must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `_-.@%=+:,*#&(){}[]` and spaces]", - }, - { - name: "userTags contains empty value", - userTags: []gcp.UserTag{{ParentID: "1234567890", Key: "key", Value: ""}}, - expectedErr: "[spec.platform.gcp.userTags[key]: Invalid value: \"\": tag value is invalid or contains invalid characters. Tag value can have a maximum of 63 characters and cannot be empty. Tag value must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `_-.@%=+:,*#&(){}[]` and spaces]", - }, - { - name: "userTags contains value with invalid character", - userTags: []gcp.UserTag{{ParentID: "1234567890", Key: "key", Value: "value*^%"}}, - expectedErr: "[spec.platform.gcp.userTags[key]: Invalid value: \"value*^%\": tag value is invalid or contains invalid characters. Tag value can have a maximum of 63 characters and cannot be empty. Tag value must begin and end with an alphanumeric character, and must contain only uppercase, lowercase alphanumeric characters, and the following special characters `_-.@%=+:,*#&(){}[]` and spaces]", - }, - { - name: "userTags contains empty ParentID", - userTags: []gcp.UserTag{{Key: "key", Value: "value*^%"}}, - expectedErr: "[spec.platform.gcp.userTags[key]: Invalid value: \"value*^%\": tag parentID is invalid or contains invalid characters. ParentID can have a maximum of 32 characters and cannot be empty. ParentID can be either OrganizationID or ProjectID. OrganizationID must consist of decimal numbers, and cannot have leading zeroes and ProjectID must be 6 to 30 characters in length, can only contain lowercase letters, numbers, and hyphens, and must start with a letter, and cannot end with a hyphen]", - }, - { - name: "userTags contains ParentID configured with invalid OrganizationID", - userTags: []gcp.UserTag{{ParentID: "00001234567890", Key: "key", Value: "value"}}, - expectedErr: "[spec.platform.gcp.userTags[key]: Invalid value: \"value\": tag parentID is invalid or contains invalid characters. ParentID can have a maximum of 32 characters and cannot be empty. ParentID can be either OrganizationID or ProjectID. OrganizationID must consist of decimal numbers, and cannot have leading zeroes and ProjectID must be 6 to 30 characters in length, can only contain lowercase letters, numbers, and hyphens, and must start with a letter, and cannot end with a hyphen]", - }, - { - name: "userTags contains ParentID configured with invalid ProjectID", - userTags: []gcp.UserTag{{ParentID: "test-project-123-", Key: "key", Value: "value"}}, - expectedErr: "[spec.platform.gcp.userTags[key]: Invalid value: \"value\": tag parentID is invalid or contains invalid characters. ParentID can have a maximum of 32 characters and cannot be empty. ParentID can be either OrganizationID or ProjectID. OrganizationID must consist of decimal numbers, and cannot have leading zeroes and ProjectID must be 6 to 30 characters in length, can only contain lowercase letters, numbers, and hyphens, and must start with a letter, and cannot end with a hyphen]", - }, - { - name: "userTags contains ParentID configured with invalid OrganizationID length", - userTags: []gcp.UserTag{ - { - ParentID: "123456789012345678901234567890123", - Key: "key", - Value: "value", - }, - }, - expectedErr: "[spec.platform.gcp.userTags[key]: Invalid value: \"value\": tag parentID is invalid or contains invalid characters. ParentID can have a maximum of 32 characters and cannot be empty. ParentID can be either OrganizationID or ProjectID. OrganizationID must consist of decimal numbers, and cannot have leading zeroes and ProjectID must be 6 to 30 characters in length, can only contain lowercase letters, numbers, and hyphens, and must start with a letter, and cannot end with a hyphen]", - }, - { - name: "userTags contains ParentID configured with invalid ProjectID length", - userTags: []gcp.UserTag{ - { - ParentID: "test-project-123-test-project-123-test-project-123-test-project-123", - Key: "key", - Value: "value", - }, - }, - expectedErr: "[spec.platform.gcp.userTags[key]: Invalid value: \"value\": tag parentID is invalid or contains invalid characters. ParentID can have a maximum of 32 characters and cannot be empty. ParentID can be either OrganizationID or ProjectID. OrganizationID must consist of decimal numbers, and cannot have leading zeroes and ProjectID must be 6 to 30 characters in length, can only contain lowercase letters, numbers, and hyphens, and must start with a letter, and cannot end with a hyphen]", - }, - } - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { - err := validateUserTags(tt.userTags, field.NewPath(fieldPath)) - if fmt.Sprintf("%v", err) != tt.expectedErr { - t.Errorf("Got: %+v Want: %+v", err, tt.expectedErr) - } - }) - } -}