Skip to content

Commit

Permalink
Clean up 'Direct' usage in resource config
Browse files Browse the repository at this point in the history
  • Loading branch information
maqiuyujoyce committed Jan 7, 2025
1 parent 974a6ea commit 61ebfd2
Show file tree
Hide file tree
Showing 25 changed files with 165 additions and 267 deletions.
30 changes: 0 additions & 30 deletions config/servicemappings/bigqueryanalyticshub.yaml

This file was deleted.

27 changes: 0 additions & 27 deletions config/servicemappings/bigqueryconnection.yaml

This file was deleted.

27 changes: 0 additions & 27 deletions config/servicemappings/bigquerydatatransfer.yaml

This file was deleted.

4 changes: 0 additions & 4 deletions config/servicemappings/cloudbuild.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,3 @@ spec:
targetField: name
ignoredFields:
- trigger_template.project_id

- name: google_cloudbuild_workerpool
kind: CloudBuildWorkerPool
direct: true
28 changes: 0 additions & 28 deletions config/servicemappings/dataform.yaml

This file was deleted.

3 changes: 0 additions & 3 deletions config/servicemappings/firestore.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 0 additions & 6 deletions config/servicemappings/kms.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 0 additions & 27 deletions config/servicemappings/networkconnectivity.yaml

This file was deleted.

27 changes: 0 additions & 27 deletions config/servicemappings/privilegedaccessmanager.yaml

This file was deleted.

4 changes: 0 additions & 4 deletions config/servicemappings/redis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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}}"
Expand Down
33 changes: 0 additions & 33 deletions config/servicemappings/workstations.yaml

This file was deleted.

65 changes: 63 additions & 2 deletions config/tests/servicemapping/servicemapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand Down Expand Up @@ -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()
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 61ebfd2

Please sign in to comment.