Skip to content

Commit

Permalink
Sanitize naming in tpgtools (#5562)
Browse files Browse the repository at this point in the history
* Fixing all the names commit.

plus, move all the name types to their own file for clarity.

* add back in a name that we actually do need which i removed too hastily.

* Go back to guessing (it's a better guess this time) the package name from the filepath.
  • Loading branch information
nat-henderson authored Jan 6, 2022
1 parent ad7edf2 commit b8ec45e
Show file tree
Hide file tree
Showing 18 changed files with 839 additions and 995 deletions.
2 changes: 1 addition & 1 deletion mmv1/third_party/terraform/utils/provider.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ end # products.each do
<% end -%>
"google_network_connectivity_hub": resourceNetworkConnectivityHub(),
"google_org_policy_policy": resourceOrgPolicyPolicy(),
"google_os_config_os_policy_assignment": resourceOSConfigOSPolicyAssignment(),
"google_os_config_os_policy_assignment": resourceOsConfigOsPolicyAssignment(),
<% if version == 'private' -%>
"google_vmware_private_cloud": resourceVmwarePrivateCloud(),
"google_vmware_cluster": resourceVmwareCluster(),
Expand Down
30 changes: 15 additions & 15 deletions tpgtools/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,11 @@ func main() {

func skipResource(r *Resource) bool {
// if a filter is specified, skip filtered services
if sFilter != nil && *sFilter != "" && *sFilter != r.ProductMetadata().PackageName {
if sFilter != nil && *sFilter != "" && DCLPackageName(*sFilter) != r.ProductMetadata().PackageName {
return true
}
// skip filtered resources
if rFilter != nil && *rFilter != "" && strings.ToLower(*rFilter) != snakeToLowercase(r.DCLName()) {
if rFilter != nil && *rFilter != "" && SnakeCaseTerraformResourceName(*rFilter) != r.Name() {
return true
}

Expand Down Expand Up @@ -158,15 +158,15 @@ func loadAndModelResources() (map[Version][]*Resource, map[Version][]*ProductMet
}

var overrideFiles []os.FileInfo
var packagePath string
var packagePath Filepath
if version == GA_VERSION {
// GA has no separate directory
packagePath = v.Name()
packagePath = Filepath(v.Name())
} else {
packagePath = path.Join(v.Name(), version.V)
packagePath = Filepath(path.Join(v.Name(), version.V))
}

overrideFiles, err = ioutil.ReadDir(path.Join(*tPath, packagePath))
overrideFiles, err = ioutil.ReadDir(path.Join(*tPath, string(packagePath)))
var newResources []*Resource

// keep track of the last document in a service- we need one for the product later
Expand Down Expand Up @@ -223,7 +223,7 @@ func addInfoExtensionsToSchemaObjects(document *openapi.Document, b []byte) erro
return nil
}

func createResourcesFromDocumentAndOverrides(document *openapi.Document, overrides Overrides, packagePath string, version Version) (resources []*Resource) {
func createResourcesFromDocumentAndOverrides(document *openapi.Document, overrides Overrides, packagePath Filepath, version Version) (resources []*Resource) {
productMetadata := GetProductMetadataFromDocument(document, packagePath)
titleParts := strings.Split(document.Info.Title, "/")

Expand Down Expand Up @@ -294,11 +294,11 @@ func generateSerializationLogic(specs map[Version][]*Resource) {
for v, resList := range specs {
for _, res := range resList {
var pkgName, pkgPath string
pkgName = res.Package() + v.SerializationSuffix
pkgName = res.Package().lowercase() + v.SerializationSuffix
if v == GA_VERSION {
pkgPath = res.Package()
pkgPath = res.Package().lowercase()
} else {
pkgPath = path.Join(res.Package(), v.V)
pkgPath = path.Join(res.Package().lowercase(), v.V)
}

if _, ok := packageMap[pkgPath]; !ok {
Expand Down Expand Up @@ -331,10 +331,10 @@ func generateSerializationLogic(specs map[Version][]*Resource) {
}
}

func loadOverrides(packagePath, fileName string) Overrides {
func loadOverrides(packagePath Filepath, fileName string) Overrides {
overrides := Overrides{}
if !(tPath == nil) && !(*tPath == "") {
b, err := ioutil.ReadFile(path.Join(*tPath, packagePath, fileName))
b, err := ioutil.ReadFile(path.Join(*tPath, string(packagePath), fileName))
if err != nil {
// ignore the error if the file just doesn't exist
if !os.IsNotExist(err) {
Expand Down Expand Up @@ -374,7 +374,7 @@ func generateResourceFile(res *Resource) {

formatted, err := formatSource(&contents)
if err != nil {
glog.Error(fmt.Errorf("error formatting %v: %v - resource \n ", res.ProductName()+res.Name(), err))
glog.Error(fmt.Errorf("error formatting %v%v: %v - resource \n ", res.ProductName(), res.Name(), err))
}

if oPath == nil || *oPath == "" {
Expand Down Expand Up @@ -415,7 +415,7 @@ func generateSweeperFile(res *Resource) {

formatted, err := formatSource(&contents)
if err != nil {
glog.Error(fmt.Errorf("error formatting %v: %v - sweeper", res.ProductName()+res.Name(), err))
glog.Error(fmt.Errorf("error formatting %v%v: %v - sweeper", res.ProductName(), res.Name(), err))
}

if oPath == nil || *oPath == "" {
Expand Down Expand Up @@ -457,7 +457,7 @@ func generateResourceTestFile(res *Resource) {

formatted, err := formatSource(&contents)
if err != nil {
glog.Error(fmt.Errorf("error formatting %v: %v - test_file \n ", res.ProductName()+res.Name(), err))
glog.Error(fmt.Errorf("error formatting %v%v: %v - test_file \n ", res.ProductName(), res.Name(), err))
}

if oPath == nil || *oPath == "" {
Expand Down
145 changes: 145 additions & 0 deletions tpgtools/names.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
package main

import "strings"

// We struggle with many types of names, and many conversions between types of names. In order to sanitize all of this,
// we have four interfaces, and many string-aliased types, which conform to at most one of the interfaces.
// This way, we can't wind up with complex, lossy, and hard-to-trace name conversions, which used to be a big problem in tpgtools.
// The functions are not really meant to be called, so much, but they are useful for converting back to regular strings where needed.
type snakeCaseName interface {
snakecase() string
}

type titleCaseName interface {
titlecase() string
}

type jsonCaseName interface {
jsoncase() string
}

type lowercaseName interface {
lowercase() string
}

// e.g. `google_compute_instance` or `google_orgpolicy_policy`.
type SnakeCaseTerraformResourceName string

func (s SnakeCaseTerraformResourceName) snakecase() string {
return string(s)
}

// e.g. `ComputeInstanceGroupManager`.
type TitleCaseFullName string

func (s TitleCaseFullName) titlecase() string {
return string(s)
}

// e.g. "compute_firewall_rule"
type SnakeCaseFullName string

func (s SnakeCaseFullName) snakecase() string {
return string(s)
}

// e.g. "os_policy"
type SnakeCaseProductName string

func (s SnakeCaseProductName) snakecase() string {
return string(s)
}

func (s SnakeCaseProductName) ToTitle() RenderedString {
return RenderedString(snakeToTitleCase(s).titlecase())
}

// e.g. "ForwardingRule"
type TitleCaseResourceName string

func (t TitleCaseResourceName) titlecase() string {
return string(t)
}

// e.g. "computeinstancegroupmanager".
type ConjoinedString string

// snakeToLowercase converts a snake_case string to a conjoined string
func snakeToLowercase(s snakeCaseName) ConjoinedString {
return ConjoinedString(strings.Join(snakeToParts(s, false), ""))
}

// snakeToTitleCase converts a snake_case string to TitleCase / Go struct case.
func snakeToTitleCase(s snakeCaseName) miscellaneousNameTitleCase {
return miscellaneousNameTitleCase(strings.Join(snakeToParts(s, true), ""))
}

// A type for a string that is not meant for further conversion. Some functions return a
// RenderedString to indicate that they have been lossily converted to another format.
type RenderedString string

func (r RenderedString) String() string {
return string(r)
}

func renderSnakeAsTitle(s snakeCaseName) RenderedString {
return RenderedString(strings.Join(snakeToParts(s, true), ""))
}

// e.g. "ospolicy"
type DCLPackageName string

func (d DCLPackageName) lowercase() string {
return string(d)
}

type BasePathOverrideNameSnakeCase string

func (b BasePathOverrideNameSnakeCase) snakecase() string {
return string(b)
}
func (b BasePathOverrideNameSnakeCase) ToUpper() RenderedString {
return RenderedString(strings.ToUpper(string(b)))
}

func (b BasePathOverrideNameSnakeCase) ToTitle() RenderedString {
title := snakeToTitleCase(b).titlecase()
// Got to special case the capitalization of "OS" in "OSConfig", for base paths specifically,
// because of interop with MMv1.
if strings.HasPrefix(string(b), "os") {
return RenderedString("OS" + title[2:])
}
return RenderedString(title)
}

// A path on the filesystem, usually relative to the root of the tpgtools/ directory.
type Filepath string

// A package path, potentially including a version suffix.
// e.g. "ospolicy/beta" or "ospolicy"
type DCLPackageNameWithVersion string

// A type for some string, not one of the things that have a specific type above, which is in
// a particular case. This is useful because we want to be able to write strings functions that take in
// a snake case string or return a snake case string, which works even if the string isn't a
// specific type.
//
// Also, having all these misc strings prevents us from winding up with a bunch of `string` types
// for things that should be explicit.
type miscellaneousNameSnakeCase string

func (m miscellaneousNameSnakeCase) snakecase() string {
return string(m)
}

type miscellaneousNameTitleCase string

func (m miscellaneousNameTitleCase) titlecase() string {
return string(m)
}

type miscellaneousNameLowercase string

func (m miscellaneousNameLowercase) lowercase() string {
return string(m)
}
4 changes: 2 additions & 2 deletions tpgtools/overrides/compute/beta/forwarding_rule.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
field: id
- type: CUSTOM_RESOURCE_NAME
details:
title: forwarding_rule
location: region
title: global_forwarding_rule
location: global
- type: CUSTOM_SCHEMA_VALUES
location: global
field: target
Expand Down
4 changes: 2 additions & 2 deletions tpgtools/overrides/compute/forwarding_rule.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
field: id
- type: CUSTOM_RESOURCE_NAME
details:
title: forwarding_rule
location: region
title: global_forwarding_rule
location: global
- type: CUSTOM_SCHEMA_VALUES
location: global
field: target
Expand Down
Loading

0 comments on commit b8ec45e

Please sign in to comment.