From 61ebfd232503d08f612bef5c858ff4c10da89f6e Mon Sep 17 00:00:00 2001 From: Joyce Ma Date: Sat, 21 Dec 2024 07:42:10 +0000 Subject: [PATCH] Clean up 'Direct' usage in resource config --- .../servicemappings/bigqueryanalyticshub.yaml | 30 -------- .../servicemappings/bigqueryconnection.yaml | 27 ------- .../servicemappings/bigquerydatatransfer.yaml | 27 ------- config/servicemappings/cloudbuild.yaml | 4 - config/servicemappings/dataform.yaml | 28 ------- config/servicemappings/firestore.yaml | 3 - config/servicemappings/kms.yaml | 6 -- .../servicemappings/networkconnectivity.yaml | 27 ------- .../privilegedaccessmanager.yaml | 27 ------- config/servicemappings/redis.yaml | 4 - config/servicemappings/workstations.yaml | 33 --------- .../servicemapping/servicemapping_test.go | 65 +++++++++++++++- .../testdata/resourcesindirectmigration.txt | 0 .../resourcedescription.go | 10 ++- pkg/gvks/supportedgvks/supportedgvks.go | 25 +++++-- pkg/krmtotf/resource.go | 6 ++ pkg/resourceskeleton/resourceskeleton_test.go | 10 +++ pkg/resourceskeleton/uri/uri.go | 5 ++ .../resourcefixture/resourcefixture_test.go | 5 ++ pkg/webhook/immutable_fields_validator.go | 4 +- scripts/generate-crds/main.go | 5 ++ .../resource-docs/iam/iampartialpolicy.md | 4 +- .../resource-docs/iam/iampolicymember.md | 2 +- .../resource-reference/main.go | 74 ++++++++++--------- .../resource-autogen/allowlist/allowlist.go | 1 - 25 files changed, 165 insertions(+), 267 deletions(-) delete mode 100644 config/servicemappings/bigqueryanalyticshub.yaml delete mode 100644 config/servicemappings/bigqueryconnection.yaml delete mode 100644 config/servicemappings/bigquerydatatransfer.yaml delete mode 100644 config/servicemappings/dataform.yaml delete mode 100644 config/servicemappings/networkconnectivity.yaml delete mode 100644 config/servicemappings/privilegedaccessmanager.yaml delete mode 100644 config/servicemappings/workstations.yaml create mode 100644 config/tests/servicemapping/testdata/resourcesindirectmigration.txt diff --git a/config/servicemappings/bigqueryanalyticshub.yaml b/config/servicemappings/bigqueryanalyticshub.yaml deleted file mode 100644 index 4513067736..0000000000 --- a/config/servicemappings/bigqueryanalyticshub.yaml +++ /dev/null @@ -1,30 +0,0 @@ -# Copyright 2024 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -apiVersion: core.cnrm.cloud.google.com/v1alpha1 -kind: ServiceMapping -metadata: - name: bigqueryanalyticshub.cnrm.cloud.google.com - namespace: cnrm-system -spec: - name: BigQueryAnalyticsHub - version: v1beta1 - serviceHostName: "analyticshub.googleapis.com" - resources: - - name: google_bigquery_analytics_hub_data_exchange - kind: BigQueryAnalyticsHubDataExchange - direct: true - - name: google_bigquery_analytics_hub_listing - kind: BigQueryAnalyticsHubListing - direct: true \ No newline at end of file diff --git a/config/servicemappings/bigqueryconnection.yaml b/config/servicemappings/bigqueryconnection.yaml deleted file mode 100644 index 424ff25faa..0000000000 --- a/config/servicemappings/bigqueryconnection.yaml +++ /dev/null @@ -1,27 +0,0 @@ -# Copyright 2024 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -apiVersion: core.cnrm.cloud.google.com/v1alpha1 -kind: ServiceMapping -metadata: - name: bigqueryconnection.cnrm.cloud.google.com - namespace: cnrm-system -spec: - name: BigQueryConnection - version: v1beta1 - serviceHostName: "bigqueryconnection.googleapis.com" - resources: - - name: google_bigquery_connection - kind: BigQueryConnectionConnection - direct: true \ No newline at end of file diff --git a/config/servicemappings/bigquerydatatransfer.yaml b/config/servicemappings/bigquerydatatransfer.yaml deleted file mode 100644 index cf2014a15a..0000000000 --- a/config/servicemappings/bigquerydatatransfer.yaml +++ /dev/null @@ -1,27 +0,0 @@ -# Copyright 2024 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -apiVersion: core.cnrm.cloud.google.com/v1alpha1 -kind: ServiceMapping -metadata: - name: bigquerydatatransfer.cnrm.cloud.google.com - namespace: cnrm-system -spec: - name: BigQueryDataTransfer - version: v1beta1 - serviceHostName: "bigquerydatatransfer.googleapis.com" - resources: - - name: google_bigquery_data_transfer_config - kind: BigQueryDataTransferConfig - direct: true diff --git a/config/servicemappings/cloudbuild.yaml b/config/servicemappings/cloudbuild.yaml index 2fc7ca84bf..38a844d99a 100644 --- a/config/servicemappings/cloudbuild.yaml +++ b/config/servicemappings/cloudbuild.yaml @@ -227,7 +227,3 @@ spec: targetField: name ignoredFields: - trigger_template.project_id - - - name: google_cloudbuild_workerpool - kind: CloudBuildWorkerPool - direct: true diff --git a/config/servicemappings/dataform.yaml b/config/servicemappings/dataform.yaml deleted file mode 100644 index d5b9393e85..0000000000 --- a/config/servicemappings/dataform.yaml +++ /dev/null @@ -1,28 +0,0 @@ -# Copyright 2024 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -apiVersion: core.cnrm.cloud.google.com/v1alpha1 -kind: ServiceMapping -metadata: - name: dataform.cnrm.cloud.google.com - namespace: cnrm-system -spec: - name: Dataform - version: v1beta1 - serviceHostName: dataform.googleapis.com - resources: - - name: google_dataform_repository - kind: DataformRepository - direct: true - v1alpha1ToV1beta1: true diff --git a/config/servicemappings/firestore.yaml b/config/servicemappings/firestore.yaml index e059be6bbb..acef203027 100644 --- a/config/servicemappings/firestore.yaml +++ b/config/servicemappings/firestore.yaml @@ -22,9 +22,6 @@ spec: version: v1beta1 serviceHostName: "firestore.googleapis.com" resources: - - name: google_firestore_database - kind: FirestoreDatabase - direct: true - name: google_firestore_index kind: FirestoreIndex serverGeneratedIDField: "name" diff --git a/config/servicemappings/kms.yaml b/config/servicemappings/kms.yaml index 29a95bc345..ecb636c5a4 100644 --- a/config/servicemappings/kms.yaml +++ b/config/servicemappings/kms.yaml @@ -70,9 +70,3 @@ spec: containers: - type: project tfField: project - - name: google_kms_autokey_config - kind: KMSAutokeyConfig - direct: true - - name: google_kms_key_handle - kind: KMSKeyHandle - direct: true \ No newline at end of file diff --git a/config/servicemappings/networkconnectivity.yaml b/config/servicemappings/networkconnectivity.yaml deleted file mode 100644 index 2bea2e84c8..0000000000 --- a/config/servicemappings/networkconnectivity.yaml +++ /dev/null @@ -1,27 +0,0 @@ -# Copyright 2024 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -apiVersion: core.cnrm.cloud.google.com/v1alpha1 -kind: ServiceMapping -metadata: - name: networkconnectivity.cnrm.cloud.google.com - namespace: cnrm-system -spec: - name: NetworkConnectivity - version: v1alpha1 - serviceHostName: networkconnectivity.googleapis.com - resources: - - name: google_network_connectivity_service_connection_policy - kind: NetworkConnectivityServiceConnectionPolicy - direct: true \ No newline at end of file diff --git a/config/servicemappings/privilegedaccessmanager.yaml b/config/servicemappings/privilegedaccessmanager.yaml deleted file mode 100644 index 5bd702479c..0000000000 --- a/config/servicemappings/privilegedaccessmanager.yaml +++ /dev/null @@ -1,27 +0,0 @@ -# Copyright 2024 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -apiVersion: core.cnrm.cloud.google.com/v1alpha1 -kind: ServiceMapping -metadata: - name: privilegedaccessmanager.cnrm.cloud.google.com - namespace: cnrm-system -spec: - name: PrivilegedAccessManager - version: v1beta1 - serviceHostName: "privilegedaccessmanager.googleapis.com" - resources: - - name: google_privileged_access_manager_entitlement - kind: PrivilegedAccessManagerEntitlement - direct: true diff --git a/config/servicemappings/redis.yaml b/config/servicemappings/redis.yaml index ab911e82d4..690a3f3549 100644 --- a/config/servicemappings/redis.yaml +++ b/config/servicemappings/redis.yaml @@ -22,10 +22,6 @@ spec: version: v1beta1 serviceHostName: "redis.googleapis.com" resources: - - name: google_redis_cluster - kind: RedisCluster - direct: true - - name: google_redis_instance kind: RedisInstance idTemplate: "{{project}}/{{region}}/{{name}}" diff --git a/config/servicemappings/workstations.yaml b/config/servicemappings/workstations.yaml deleted file mode 100644 index 4a0c24de3d..0000000000 --- a/config/servicemappings/workstations.yaml +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright 2024 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -apiVersion: core.cnrm.cloud.google.com/v1alpha1 -kind: ServiceMapping -metadata: - name: workstations.cnrm.cloud.google.com - namespace: cnrm-system -spec: - name: Workstations - version: v1beta1 - serviceHostName: "workstations.googleapis.com" - resources: - - name: google_workstations_workstation - kind: Workstation - direct: true - - name: google_workstations_workstationcluster - kind: WorkstationCluster - direct: true - - name: google_workstations_workstationconfig - kind: WorkstationConfig - direct: true \ No newline at end of file diff --git a/config/tests/servicemapping/servicemapping_test.go b/config/tests/servicemapping/servicemapping_test.go index d7ba4c7348..6533bfda89 100644 --- a/config/tests/servicemapping/servicemapping_test.go +++ b/config/tests/servicemapping/servicemapping_test.go @@ -27,6 +27,7 @@ import ( "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/gvks/supportedgvks" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/krmtotf" + "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/test" testservicemappingloader "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/test/servicemappingloader" tfprovider "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/tf/provider" tfresource "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/tf/resource" @@ -47,6 +48,11 @@ func TestIDTemplateCanBeUsedToMatchResourceNameShouldHaveValue(t *testing.T) { serviceMappings := testservicemappingloader.New(t).GetServiceMappings() for _, sm := range serviceMappings { for _, rc := range sm.Spec.Resources { + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if rc.Direct { continue } @@ -99,6 +105,45 @@ func TestServiceHostName(t *testing.T) { } } +func TestDirectResourceConfigsForMigrationOnly(t *testing.T) { + smLoader := testservicemappingloader.New(t) + var allDirectRCs []string + gvks := k8s.SortGVKsByKind(supportedgvks.BasedOnAllServiceMappings(smLoader)) + for _, gvk := range gvks { + rcs, err := smLoader.GetResourceConfigs(gvk) + if err != nil { + t.Fatalf("error getting resource config for gvk %+v", gvk) + } + if len(rcs) == 0 { + t.Errorf("no resource config for gvk %+v but there should be at least one", gvk) + } + directCount := 0 + var directRCs []string + for _, rc := range rcs { + if rc.Direct { + directCount++ + + // Only a Kind that is still TF-based can have `direct: true` in + // its resource config(s). + if !supportedgvks.IsTFBasedByGVK(gvk) { + t.Errorf("gvk %+v (resource config %v) is set to direct but it is not in direct migration", gvk, rc.Name) + } + directRCs = append(directRCs, fmt.Sprintf("gvk=%+v, resourceConfigName=%v", gvk, rc.Name)) + } + } + if directCount == len(rcs) { + allDirectRCs = append(allDirectRCs, directRCs...) + } else if directCount == 0 { + continue + } else { + t.Errorf("%v out of %v resource configs mapping to gvk %+v is/are direct, but all should be direct or none should be direct", directCount, len(rcs), gvk) + } + } + + want := strings.Join(allDirectRCs, "\n") + test.CompareGoldenFile(t, "testdata/resourcesindirectmigration.txt", want) +} + func TestIAMPolicyMappings(t *testing.T) { t.Parallel() serviceMappings := testservicemappingloader.New(t).GetServiceMappings() @@ -110,6 +155,11 @@ func TestIAMPolicyMappings(t *testing.T) { if rc.Kind == "ComputeBackendService" { continue } + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if rc.Direct { // Do not check for direct resource continue } @@ -612,6 +662,11 @@ func TestMustHaveIDTemplateOrServerGeneratedId(t *testing.T) { } func assertIDTemplateOrServerGeneratedID(t *testing.T, rc v1alpha1.ResourceConfig) { + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if !rc.Direct && rc.IDTemplate == "" && rc.ServerGeneratedIDField == "" { t.Fatalf("resource kind '%v' with name '%v' has neither id template or server generated ID defined: at least one must be present", rc.Kind, rc.Name) } @@ -1270,8 +1325,14 @@ func TestStorageVersionIsSetAndValidIFFV1alpha1ToV1beta1IsSet(t *testing.T) { continue } if isV1alpha1ToV1beta1 { - // if this is a direct resource, the storage version is defiend - // in the kubebuilder tooling + // If this is a direct resource, the storage version is defined + // in the kubebuilder tooling. + // + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if r.Direct { continue } diff --git a/config/tests/servicemapping/testdata/resourcesindirectmigration.txt b/config/tests/servicemapping/testdata/resourcesindirectmigration.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pkg/cli/cmd/printresources/resourcedescription/resourcedescription.go b/pkg/cli/cmd/printresources/resourcedescription/resourcedescription.go index 19e58c4eee..ecba7b84ca 100644 --- a/pkg/cli/cmd/printresources/resourcedescription/resourcedescription.go +++ b/pkg/cli/cmd/printresources/resourcedescription/resourcedescription.go @@ -102,7 +102,15 @@ func doesResourceSupportExport(tfProvider *tfschema.Provider, sm v1alpha1.Servic } func resourceHasTFImporter(rc v1alpha1.ResourceConfig, tfProvider *tfschema.Provider) bool { - // every value for rc.Name should be in the ResourcesMap + // Every value for rc.Name should be in the ResourcesMap. + // + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. + // Once a resource is migrated to direct, we need to figure out a different + // way to support export for backwards compatibility if needed. if rc.Direct { return false } diff --git a/pkg/gvks/supportedgvks/supportedgvks.go b/pkg/gvks/supportedgvks/supportedgvks.go index 9560076683..9ad125966d 100644 --- a/pkg/gvks/supportedgvks/supportedgvks.go +++ b/pkg/gvks/supportedgvks/supportedgvks.go @@ -15,8 +15,6 @@ package supportedgvks import ( - "fmt" - iamapi "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/apis/iam/v1beta1" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/dcl/metadata" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s" @@ -45,10 +43,7 @@ func ManualResources(smLoader *servicemappingloader.ServiceMappingLoader, servic func resourcesWithDirect(smLoader *servicemappingloader.ServiceMappingLoader, serviceMetaLoader metadata.ServiceMetadataLoader, includesAutoGen bool) ([]schema.GroupVersionKind, error) { gvks := resources(smLoader, serviceMetaLoader, includesAutoGen) - directGVKs, err := DirectResources() - if err != nil { - return nil, fmt.Errorf("error getting direct resource GVKs: %w", err) - } + directGVKs := DirectResources() for _, gvk := range gvks { if _, ok := directGVKs[gvk]; ok { delete(directGVKs, gvk) @@ -66,7 +61,7 @@ func resources(smLoader *servicemappingloader.ServiceMappingLoader, serviceMetaL return gvks } -func DirectResources() (map[schema.GroupVersionKind]bool, error) { +func DirectResources() map[schema.GroupVersionKind]bool { handWrittenIAMTypes := make(map[schema.GroupVersionKind]bool) directResources := make(map[schema.GroupVersionKind]bool) for _, gvk := range BasedOnHandwrittenIAMTypes() { @@ -84,7 +79,7 @@ func DirectResources() (map[schema.GroupVersionKind]bool, error) { } directResources[gvk] = true } - return directResources, nil + return directResources } // AllDynamicTypes returns GroupVersionKinds generated from: @@ -158,3 +153,17 @@ func IsDirectByGVK(gvk schema.GroupVersionKind) bool { } return true } + +func IsTFBasedByGVK(gvk schema.GroupVersionKind) bool { + metadata, ok := SupportedGVKs[gvk] + if !ok { + return false + } + if metadata.Labels[k8s.TF2CRDLabel] == "true" { + return true + } + if metadata.Labels[k8s.DCL2CRDLabel] == "true" { + return false + } + return false +} diff --git a/pkg/krmtotf/resource.go b/pkg/krmtotf/resource.go index d41fa040d1..57760ce5fc 100644 --- a/pkg/krmtotf/resource.go +++ b/pkg/krmtotf/resource.go @@ -80,6 +80,12 @@ func NewResource(u *unstructured.Unstructured, sm *corekccv1alpha1.ServiceMappin func NewResourceFromResourceConfig(rc *corekccv1alpha1.ResourceConfig, p *tfschema.Provider) (*Resource, error) { tfResource, ok := p.ResourcesMap[rc.Name] // Pure Direct Resource does not have ResourceMap. + // + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if rc.Direct { return &Resource{ TFInfo: &terraform.InstanceInfo{ diff --git a/pkg/resourceskeleton/resourceskeleton_test.go b/pkg/resourceskeleton/resourceskeleton_test.go index 089cc5294a..4e7fde0d0e 100644 --- a/pkg/resourceskeleton/resourceskeleton_test.go +++ b/pkg/resourceskeleton/resourceskeleton_test.go @@ -124,6 +124,11 @@ func ensureAssetTCExistsForEachResourceConfig(t *testing.T, smLoader *servicemap } for _, sm := range smLoader.GetServiceMappings() { for _, rc := range sm.Spec.Resources { + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if rc.Direct { continue } @@ -147,6 +152,11 @@ func ensureURITCExistsForEachResourceConfig(t *testing.T, smLoader *servicemappi } for _, sm := range smLoader.GetServiceMappings() { for _, rc := range sm.Spec.Resources { + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if rc.Direct { continue } diff --git a/pkg/resourceskeleton/uri/uri.go b/pkg/resourceskeleton/uri/uri.go index c74b6439ce..5d398fc9fa 100644 --- a/pkg/resourceskeleton/uri/uri.go +++ b/pkg/resourceskeleton/uri/uri.go @@ -43,6 +43,11 @@ func matchResourceNameToRC(uriPath string, sm *v1alpha1.ServiceMapping) (*v1alph func matchResourceNameToRCGeneral(uriPath string, sm *v1alpha1.ServiceMapping) (*v1alpha1.ResourceConfig, error) { for _, rc := range sm.Spec.Resources { + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if rc.Direct { continue } diff --git a/pkg/test/resourcefixture/resourcefixture_test.go b/pkg/test/resourcefixture/resourcefixture_test.go index e3eb40ef91..88c886577b 100644 --- a/pkg/test/resourcefixture/resourcefixture_test.go +++ b/pkg/test/resourcefixture/resourcefixture_test.go @@ -75,6 +75,11 @@ func getResourceConfigIDSet(t *testing.T, smLoader *servicemappingloader.Service resourceConfigIds := make(map[string]bool, 0) for _, sm := range sms { for _, rc := range sm.Spec.Resources { + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if rc.Direct { continue } diff --git a/pkg/webhook/immutable_fields_validator.go b/pkg/webhook/immutable_fields_validator.go index 94d3376358..31181c117c 100644 --- a/pkg/webhook/immutable_fields_validator.go +++ b/pkg/webhook/immutable_fields_validator.go @@ -31,6 +31,7 @@ import ( dclcontainer "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/dcl/extension/container" dclmetadata "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/dcl/metadata" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/dcl/schema/dclschemaloader" + "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/gvks/supportedgvks" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/krmtotf" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/servicemapping/servicemappingloader" @@ -294,7 +295,8 @@ func validateImmutableFieldsForTFBasedResource(obj, oldObj *unstructured.Unstruc return admission.Errored(http.StatusBadRequest, fmt.Errorf("couldn't get ResourceConfig for kind %v: %w", obj.GetKind(), err)) } - if rc.Direct && rc.Name != "google_sql_database_instance" { + isDirect := supportedgvks.IsDirectByGVK(obj.GroupVersionKind()) + if isDirect && rc.Name != "google_sql_database_instance" { return allowedResponse } diff --git a/scripts/generate-crds/main.go b/scripts/generate-crds/main.go index f1bb9e73d5..7a7785c26a 100644 --- a/scripts/generate-crds/main.go +++ b/scripts/generate-crds/main.go @@ -124,6 +124,11 @@ func generateTFBasedCRDs() []*apiextensions.CustomResourceDefinition { crds := make([]*apiextensions.CustomResourceDefinition, 0) directCount := 0 for _, rc := range rcs { + // TODO: remove 'Direct' field from ResourceConfig and remove the if statement. + // The 'Direct' indicator won't be needed after we finish all the migrations. + // The 'Direct' indicator is necessary during the migration so + // that Config Connector uses direct approach to generate CRDs + // but still allow TF-based controller to reconcile the resource. if rc.Direct { fmt.Printf("skip generate TF-based CRD for direct resource %s\n", rc.Kind) directCount += 1 diff --git a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/iam/iampartialpolicy.md b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/iam/iampartialpolicy.md index ad1a271423..ed129d6315 100644 --- a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/iam/iampartialpolicy.md +++ b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/iam/iampartialpolicy.md @@ -1108,7 +1108,7 @@ observedGeneration: integer ## Sample YAML(s) -### Project Level Iampartialpolicy +### Project Level IAMPartialPolicy ```yaml # Copyright 2020 Google LLC # @@ -1164,7 +1164,7 @@ spec: external: "${ORG_ID?}" ``` -### PubSub Admin Iampartialpolicy +### PubSub Admin IAMPartialPolicy ```yaml # Copyright 2020 Google LLC # diff --git a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/iam/iampolicymember.md b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/iam/iampolicymember.md index 721426e31f..a1a168dc8b 100644 --- a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/iam/iampolicymember.md +++ b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/iam/iampolicymember.md @@ -1091,7 +1091,7 @@ metadata: name: iampolicymember-dep-orgrole ``` -### Policy Member With BigQueryConnection Reference +### Policy Member With Bigqueryconnection Reference ```yaml # Copyright 2024 Google LLC # diff --git a/scripts/generate-google3-docs/resource-reference/main.go b/scripts/generate-google3-docs/resource-reference/main.go index ebe13556ea..dc8125a367 100644 --- a/scripts/generate-google3-docs/resource-reference/main.go +++ b/scripts/generate-google3-docs/resource-reference/main.go @@ -139,8 +139,7 @@ var ( ) var ( - dclSchemaLoader dclschemaloader.DCLSchemaLoader - serviceMetadataLoader dclmetadata.ServiceMetadataLoader + dclSchemaLoader dclschemaloader.DCLSchemaLoader ) func main() { @@ -156,18 +155,16 @@ func main() { if err != nil { log.Fatal(fmt.Errorf("error creating a DCL schema loader: %w", err)) } - serviceMetadataLoader = dclmetadata.New() + serviceMetadataLoader := dclmetadata.New() manualResources, err := supportedgvks.ManualResources(smLoader, serviceMetadataLoader) if err != nil { log.Fatalf("error getting manual resources: %v", err) } - directGVKs, err := supportedgvks.DirectResources() - if err != nil { - log.Fatalf("error getting direct resource GVKs: %v", err) - } + directGVKs := supportedgvks.DirectResources() docGenerator := &DocGenerator{ - smLoader: smLoader, - directGVKs: directGVKs, + smLoader: smLoader, + serviceMetadataLoader: serviceMetadataLoader, + directGVKs: directGVKs, } for _, gvk := range manualResources { if strings.HasPrefix(gvk.Version, "v1alpha") { @@ -181,8 +178,9 @@ func main() { } type DocGenerator struct { - smLoader *servicemappingloader.ServiceMappingLoader - directGVKs map[schema.GroupVersionKind]bool + smLoader *servicemappingloader.ServiceMappingLoader + serviceMetadataLoader dclmetadata.ServiceMetadataLoader + directGVKs map[schema.GroupVersionKind]bool } func (d *DocGenerator) generateDocForGVK(gvk schema.GroupVersionKind) error { @@ -299,18 +297,18 @@ func (d *DocGenerator) constructResourceForGVK(gvk schema.GroupVersionKind) (*re if err = buildFieldDescriptions(r, crd, gvk.Version); err != nil { return nil, fmt.Errorf("buildFieldDescriptions: %w", err) } - r.DefaultReconcileInterval = uint32(reconciliationinterval.MeanReconcileReenqueuePeriod(gvk, d.smLoader, serviceMetadataLoader).Seconds()) + r.DefaultReconcileInterval = uint32(reconciliationinterval.MeanReconcileReenqueuePeriod(gvk, d.smLoader, d.serviceMetadataLoader).Seconds()) isDirectGVK := d.directGVKs[gvk] if err != nil { return nil, fmt.Errorf("error checking whether GVK is direct: %w", err) } - if dclmetadata.IsDCLBasedResourceKind(gvk, serviceMetadataLoader) { - resourceMetadata, found := serviceMetadataLoader.GetResourceWithGVK(gvk) + if dclmetadata.IsDCLBasedResourceKind(gvk, d.serviceMetadataLoader) { + resourceMetadata, found := d.serviceMetadataLoader.GetResourceWithGVK(gvk) if !found { return nil, fmt.Errorf("error finding the DCL based resource %v", gvk) } r.IsAlphaResource = resourceMetadata.DCLVersion == "alpha" - if err := handleAnnotationsAndIAMSettingsForDCLBasedResource(r, gvk); err != nil { + if err := d.handleAnnotationsAndIAMSettingsForDCLBasedResource(r, gvk); err != nil { return nil, fmt.Errorf("error processing the DCL based resource %v: %w", gvk, err) } } else { @@ -338,13 +336,13 @@ func (d *DocGenerator) constructResourceForGVK(gvk schema.GroupVersionKind) (*re return r, nil } -func handleAnnotationsAndIAMSettingsForDCLBasedResource(r *resource, gvk schema.GroupVersionKind) error { +func (d *DocGenerator) handleAnnotationsAndIAMSettingsForDCLBasedResource(r *resource, gvk schema.GroupVersionKind) error { annotationSet := sets.NewString() - resourceMetadata, found := serviceMetadataLoader.GetResourceWithGVK(gvk) + resourceMetadata, found := d.serviceMetadataLoader.GetResourceWithGVK(gvk) if !found { return fmt.Errorf("ServiceMetadata for resource with GroupVersionKind %v not found", gvk) } - containers, err := dclcontainer.GetContainersForGVK(gvk, serviceMetadataLoader, dclSchemaLoader) + containers, err := dclcontainer.GetContainersForGVK(gvk, d.serviceMetadataLoader, dclSchemaLoader) if err != nil { return err } @@ -356,7 +354,7 @@ func handleAnnotationsAndIAMSettingsForDCLBasedResource(r *resource, gvk schema. } } setAnnotations(r, annotationSet) - externalReferenceFormat, err := getDCLExternalReferenceFormatIfSupportsIAM(gvk) + externalReferenceFormat, err := d.getDCLExternalReferenceFormatIfSupportsIAM(gvk) if err != nil { return err } @@ -469,8 +467,8 @@ func iamExternalReferenceFormatsFor(gvk schema.GroupVersionKind, rcs []*v1alpha1 } } -func getDCLExternalReferenceFormatIfSupportsIAM(gvk schema.GroupVersionKind) (string, error) { - dclSchema, err := dclschemaloader.GetDCLSchemaForGVK(gvk, serviceMetadataLoader, dclSchemaLoader) +func (d *DocGenerator) getDCLExternalReferenceFormatIfSupportsIAM(gvk schema.GroupVersionKind) (string, error) { + dclSchema, err := dclschemaloader.GetDCLSchemaForGVK(gvk, d.serviceMetadataLoader, dclSchemaLoader) if err != nil { return "", err } @@ -502,7 +500,10 @@ func (d *DocGenerator) buildSamples(kind, sampleDirPath string) (map[string]stri if err != nil { return nil, fmt.Errorf("error building sample at %v: %w", subDirPath, err) } - name := d.titleForSample(subDir) + name, err := d.titleForSample(subDir) + if err != nil { + return nil, fmt.Errorf("error constructing sample title at %v: %w", subDirPath, err) + } sampleYAMLs[name] = sample } } else { @@ -666,8 +667,8 @@ func (d *DocGenerator) referencesSupportedByIAMPolicy() ([]iamPolicyReference, e ExternalReferenceFormats: externalReferenceFormats, }) } - for _, gvk := range supportedgvks.BasedOnDCL(serviceMetadataLoader) { - externalReferenceFormat, err := getDCLExternalReferenceFormatIfSupportsIAM(gvk) + for _, gvk := range supportedgvks.BasedOnDCL(d.serviceMetadataLoader) { + externalReferenceFormat, err := d.getDCLExternalReferenceFormatIfSupportsIAM(gvk) if err != nil { return nil, err } @@ -725,8 +726,8 @@ func (d *DocGenerator) referencesSupportedByIAMPolicyMember() ([]iamPolicyMember ExternalReferenceFormats: externalReferenceFormats, }) } - for _, gvk := range supportedgvks.BasedOnDCL(serviceMetadataLoader) { - externalReferenceFormat, err := getDCLExternalReferenceFormatIfSupportsIAM(gvk) + for _, gvk := range supportedgvks.BasedOnDCL(d.serviceMetadataLoader) { + externalReferenceFormat, err := d.getDCLExternalReferenceFormatIfSupportsIAM(gvk) if err != nil { return nil, err } @@ -788,15 +789,18 @@ func (d *DocGenerator) referencesSupportedByIAMAuditConfig() ([]iamAuditConfigRe } // titleForSample returns the title that should be used for the sample -func (d *DocGenerator) titleForSample(dir fs.FileInfo) string { +func (d *DocGenerator) titleForSample(dir fs.FileInfo) (string, error) { serviceMappingNames := make(map[string]v1alpha1.ServiceMapping) - resourceKinds := make(map[string]v1alpha1.ResourceConfig) + resourceKinds := make(map[string]string) for _, sm := range d.smLoader.GetServiceMappings() { serviceMappingNames[strings.ToLower(sm.Spec.Name)] = sm - - for _, resource := range sm.Spec.Resources { - resourceKinds[strings.ToLower(resource.Kind)] = resource - } + } + gvks, err := supportedgvks.All(d.smLoader, d.serviceMetadataLoader) + if err != nil { + return "", err + } + for _, gvk := range gvks { + resourceKinds[strings.ToLower(gvk.Kind)] = gvk.Kind } split := strings.Split(dir.Name(), "-") var words []string @@ -806,9 +810,9 @@ func (d *DocGenerator) titleForSample(dir fs.FileInfo) string { // If it's a well-known service, use its correctly capitalized name (PubSub, VertexAI etc) word = serviceMapping.Spec.Name } - if resource, ok := resourceKinds[strings.ToLower(word)]; ok { + if resourceKind, ok := resourceKinds[strings.ToLower(word)]; ok { // If it's a well-known kind, use the correct capitalization - word = resource.Kind + word = resourceKind } for _, s := range allowedSpellings { if strings.EqualFold(s, word) { @@ -817,7 +821,7 @@ func (d *DocGenerator) titleForSample(dir fs.FileInfo) string { } words = append(words, word) } - return strings.Join(words, " ") + return strings.Join(words, " "), nil } func resourceSupportsIAMPolicyAndPolicyMember(rc *v1alpha1.ResourceConfig) bool { diff --git a/scripts/resource-autogen/allowlist/allowlist.go b/scripts/resource-autogen/allowlist/allowlist.go index f467bf24aa..f30ff50cf1 100644 --- a/scripts/resource-autogen/allowlist/allowlist.go +++ b/scripts/resource-autogen/allowlist/allowlist.go @@ -95,7 +95,6 @@ var ( "data_catalog/google_data_catalog_entry_group", "data_catalog/google_data_catalog_tag", "data_catalog/google_data_catalog_tag_template", - "dataform/google_dataform_repository", "datastore/google_datastore_index", "datastream/google_datastream_connection_profile", "datastream/google_datastream_private_connection",