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 Dec 21, 2024
1 parent 7eaca66 commit b35efdc
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 13 deletions.
8 changes: 8 additions & 0 deletions config/tests/servicemapping/servicemapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
11 changes: 3 additions & 8 deletions pkg/gvks/supportedgvks/supportedgvks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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() {
Expand All @@ -84,7 +79,7 @@ func DirectResources() (map[schema.GroupVersionKind]bool, error) {
}
directResources[gvk] = true
}
return directResources, nil
return directResources
}

// AllDynamicTypes returns GroupVersionKinds generated from:
Expand Down
4 changes: 4 additions & 0 deletions pkg/krmtotf/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 4 additions & 0 deletions pkg/resourceskeleton/resourceskeleton_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/resourceskeleton/uri/uri.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/test/resourcefixture/resourcefixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/webhook/immutable_fields_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 5 additions & 0 deletions scripts/generate-crds/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions scripts/generate-google3-docs/resource-reference/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit b35efdc

Please sign in to comment.