diff --git a/config/tests/servicemapping/servicemapping_test.go b/config/tests/servicemapping/servicemapping_test.go index d7ba4c7348..dc0a41b161 100644 --- a/config/tests/servicemapping/servicemapping_test.go +++ b/config/tests/servicemapping/servicemapping_test.go @@ -47,6 +47,8 @@ func TestIDTemplateCanBeUsedToMatchResourceNameShouldHaveValue(t *testing.T) { serviceMappings := testservicemappingloader.New(t).GetServiceMappings() for _, sm := range serviceMappings { for _, rc := range sm.Spec.Resources { + // TODO: remove direct resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. if rc.Direct { continue } @@ -110,6 +112,8 @@ func TestIAMPolicyMappings(t *testing.T) { if rc.Kind == "ComputeBackendService" { continue } + // TODO: remove direct resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. if rc.Direct { // Do not check for direct resource continue } @@ -612,6 +616,8 @@ func TestMustHaveIDTemplateOrServerGeneratedId(t *testing.T) { } func assertIDTemplateOrServerGeneratedID(t *testing.T, rc v1alpha1.ResourceConfig) { + // TODO: remove direct resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. 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,6 +1276,8 @@ func TestStorageVersionIsSetAndValidIFFV1alpha1ToV1beta1IsSet(t *testing.T) { continue } if isV1alpha1ToV1beta1 { + // TODO: remove direct resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. // if this is a direct resource, the storage version is defiend // in the kubebuilder tooling if r.Direct { diff --git a/pkg/cli/cmd/printresources/resourcedescription/resourcedescription.go b/pkg/cli/cmd/printresources/resourcedescription/resourcedescription.go index 19e58c4eee..eecee17429 100644 --- a/pkg/cli/cmd/printresources/resourcedescription/resourcedescription.go +++ b/pkg/cli/cmd/printresources/resourcedescription/resourcedescription.go @@ -103,6 +103,10 @@ 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 + // TODO: remove direct resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. + // This means once a resource is migrated to direct, we need to figure out + // a similar 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..a8a0444a8f 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: diff --git a/pkg/krmtotf/resource.go b/pkg/krmtotf/resource.go index d41fa040d1..c33648cc22 100644 --- a/pkg/krmtotf/resource.go +++ b/pkg/krmtotf/resource.go @@ -80,6 +80,10 @@ 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 resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. + // This means once a resource is migrated to direct, it errors out when + // the resource is still reconciled via the TF-based controller. if rc.Direct { return &Resource{ TFInfo: &terraform.InstanceInfo{ diff --git a/pkg/resourceskeleton/resourceskeleton_test.go b/pkg/resourceskeleton/resourceskeleton_test.go index 089cc5294a..c864548b76 100644 --- a/pkg/resourceskeleton/resourceskeleton_test.go +++ b/pkg/resourceskeleton/resourceskeleton_test.go @@ -124,6 +124,8 @@ func ensureAssetTCExistsForEachResourceConfig(t *testing.T, smLoader *servicemap } for _, sm := range smLoader.GetServiceMappings() { for _, rc := range sm.Spec.Resources { + // TODO: remove direct resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. if rc.Direct { continue } @@ -147,6 +149,8 @@ func ensureURITCExistsForEachResourceConfig(t *testing.T, smLoader *servicemappi } for _, sm := range smLoader.GetServiceMappings() { for _, rc := range sm.Spec.Resources { + // TODO: remove direct resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. if rc.Direct { continue } diff --git a/pkg/resourceskeleton/uri/uri.go b/pkg/resourceskeleton/uri/uri.go index c74b6439ce..9aaa9c9de4 100644 --- a/pkg/resourceskeleton/uri/uri.go +++ b/pkg/resourceskeleton/uri/uri.go @@ -43,6 +43,8 @@ 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 resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. if rc.Direct { continue } diff --git a/pkg/test/resourcefixture/resourcefixture_test.go b/pkg/test/resourcefixture/resourcefixture_test.go index e3eb40ef91..f0ab745a48 100644 --- a/pkg/test/resourcefixture/resourcefixture_test.go +++ b/pkg/test/resourcefixture/resourcefixture_test.go @@ -75,6 +75,8 @@ 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 resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. 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..94e40c1d18 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 resources from service mapping and remove the if statement. + // This if statement won't be true once all direct resources are removed. + // However, we probably need a "useDirectCRD" variable in RC to + // use direct approach to generate CRDs but still allow TF-based + // controller to reconcile the resource during the migration. 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/main.go b/scripts/generate-google3-docs/resource-reference/main.go index ebe13556ea..5b0a5f240f 100644 --- a/scripts/generate-google3-docs/resource-reference/main.go +++ b/scripts/generate-google3-docs/resource-reference/main.go @@ -161,10 +161,7 @@ func main() { 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,