[WIP] Clean up 'Direct' usage in resource config

config/servicemappings/cloudbuild.yaml

@@ -227,7 +227,3 @@ spec:
           targetField: name
       ignoredFields:
         - trigger_template.project_id
-
-    - name: google_cloudbuild_workerpool
-      kind: CloudBuildWorkerPool
-      direct: true

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"

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

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}}"

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
 				}

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
 	}