From 126f97c4ccfa09981ada1b3cd932fd6628aa19cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Fri, 25 Sep 2026 15:39:49 +0200 Subject: [PATCH] Fix naive pluralization in scaffolding tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace all naive "+s" plural suffixes in scaffold-controller and resource-generator templates with a pluralize() function that handles common English rules: - Consonant + y → ies (e.g. Policy → Policies) - Sibilant endings (s, sh, ch, x, z) → es - Default → s This prevents incorrect forms like "qospolicys" when scaffolding resources whose names don't simply take an s. --- .../config-crd-kustomization.yaml.template | 2 +- cmd/resource-generator/main.go | 36 ++++++++- cmd/resource-generator/main_test.go | 63 +++++++++++++++ .../data/client/client.go.template | 8 +- .../data/controller/actuator.go.template | 4 +- .../data/controller/controller.go.template | 4 +- .../data/tests/dependency/README.md.template | 6 +- .../tests/import-error/README.md.template | 2 +- cmd/scaffold-controller/main.go | 33 ++++++++ cmd/scaffold-controller/main_test.go | 78 +++++++++++++++++++ 10 files changed, 222 insertions(+), 14 deletions(-) create mode 100644 cmd/resource-generator/main_test.go create mode 100644 cmd/scaffold-controller/main_test.go diff --git a/cmd/resource-generator/data/config-crd-kustomization.yaml.template b/cmd/resource-generator/data/config-crd-kustomization.yaml.template index 96a83223a..c86de10bd 100644 --- a/cmd/resource-generator/data/config-crd-kustomization.yaml.template +++ b/cmd/resource-generator/data/config-crd-kustomization.yaml.template @@ -3,7 +3,7 @@ # It should be run by config/default resources: {{- range . }} -- bases/openstack.k-orc.cloud_{{ .NameLower }}s.yaml +- bases/openstack.k-orc.cloud_{{ .NameLower | plural }}.yaml {{- end}} # +kubebuilder:scaffold:crdkustomizeresource diff --git a/cmd/resource-generator/main.go b/cmd/resource-generator/main.go index 1c0a70e74..b159bdb84 100644 --- a/cmd/resource-generator/main.go +++ b/cmd/resource-generator/main.go @@ -257,7 +257,9 @@ func main() { controllerTemplate := template.Must(template.New("controller").Parse(controller_template)) projectTemplate := template.Must(template.New("project").Parse(project_template)) kuttlTestTemplate := template.Must(template.New("kuttl-test").Parse(kuttl_test_template)) - crdKustomizationTemplate := template.Must(template.New("crd-kustomization").Parse(crd_kustomization_template)) + funcMap := template.FuncMap{"plural": pluralize} + crdKustomizationTemplate := template.Must( + template.New("crd-kustomization").Funcs(funcMap).Parse(crd_kustomization_template)) samplesKustomizationTemplate := template.Must( template.New("samples-kustomization").Parse(samples_kustomization_template)) mockDocTemplate := template.Must(template.New("mock-doc").Parse(mock_doc_template)) @@ -356,6 +358,38 @@ func writeTemplate[T ResourceType](path string, tmpl *template.Template, resourc return tmpl.Execute(file, resource) } +// pluralize returns the English plural of the given word. +// It handles common suffixes: consonant+y → ies, sibilants → es, +// and falls back to appending s. +func pluralize(s string) string { + if s == "" { + return s + } + + lower := strings.ToLower(s) + + // Words ending in a consonant followed by "y": replace "y" with "ies" + if strings.HasSuffix(lower, "y") { + // Check the character before 'y' is a consonant (not a vowel) + if len(lower) >= 2 { + beforeY := lower[len(lower)-2] + if !strings.ContainsRune("aeiou", rune(beforeY)) { + return s[:len(s)-1] + "ies" + } + } + } + + // Words ending in s, sh, ch, x, z: append "es" + for _, suffix := range []string{"s", "sh", "ch", "x", "z"} { + if strings.HasSuffix(lower, suffix) { + return s + "es" + } + } + + // Default: append "s" + return s + "s" +} + func writeAutogeneratedHeader(f *os.File) error { var commentPrefix string diff --git a/cmd/resource-generator/main_test.go b/cmd/resource-generator/main_test.go new file mode 100644 index 000000000..c42f71053 --- /dev/null +++ b/cmd/resource-generator/main_test.go @@ -0,0 +1,63 @@ +package main + +import "testing" + +func TestPluralize(t *testing.T) { + tests := []struct { + input string + want string + }{ + // Empty string + {"", ""}, + + // All existing ORC resources (lowercase, as used by NameLower) + {"addressscope", "addressscopes"}, + {"applicationcredential", "applicationcredentials"}, + {"domain", "domains"}, + {"endpoint", "endpoints"}, + {"flavor", "flavors"}, + {"floatingip", "floatingips"}, + {"group", "groups"}, + {"image", "images"}, + {"keypair", "keypairs"}, + {"limit", "limits"}, + {"network", "networks"}, + {"port", "ports"}, + {"project", "projects"}, + {"region", "regions"}, + {"registeredlimit", "registeredlimits"}, + {"role", "roles"}, + {"roleassignment", "roleassignments"}, + {"router", "routers"}, + {"routerinterface", "routerinterfaces"}, + {"securitygroup", "securitygroups"}, + {"server", "servers"}, + {"servergroup", "servergroups"}, + {"service", "services"}, + {"sharenetwork", "sharenetworks"}, + {"subnet", "subnets"}, + {"trunk", "trunks"}, + {"user", "users"}, + {"volume", "volumes"}, + {"volumetype", "volumetypes"}, + + // Consonant + y → ies + {"policy", "policies"}, + {"qospolicy", "qospolicies"}, + + // Vowel + y → just s + {"key", "keys"}, + + // Sibilant endings → es + {"address", "addresses"}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := pluralize(tt.input) + if got != tt.want { + t.Errorf("pluralize(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} diff --git a/cmd/scaffold-controller/data/client/client.go.template b/cmd/scaffold-controller/data/client/client.go.template index bba2af076..01689dc5f 100644 --- a/cmd/scaffold-controller/data/client/client.go.template +++ b/cmd/scaffold-controller/data/client/client.go.template @@ -28,7 +28,7 @@ import ( ) type {{ .Kind }}Client interface { - List{{ .Kind }}s(ctx context.Context, listOpts {{ .GophercloudPackage }}.ListOptsBuilder) iter.Seq2[*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error] + List{{ .Kind | plural }}(ctx context.Context, listOpts {{ .GophercloudPackage }}.ListOptsBuilder) iter.Seq2[*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error] Create{{ .Kind }}(ctx context.Context, opts {{ .GophercloudPackage }}.CreateOptsBuilder) (*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error) Delete{{ .Kind }}(ctx context.Context, resourceID string) error Get{{ .Kind }}(ctx context.Context, resourceID string) (*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error) @@ -51,10 +51,10 @@ func New{{ .Kind }}Client(providerClient *gophercloud.ProviderClient, providerCl return &{{ .PackageName }}Client{client}, nil } -func (c {{ .PackageName }}Client) List{{ .Kind }}s(ctx context.Context, listOpts {{ .GophercloudPackage }}.ListOptsBuilder) iter.Seq2[*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error] { +func (c {{ .PackageName }}Client) List{{ .Kind | plural }}(ctx context.Context, listOpts {{ .GophercloudPackage }}.ListOptsBuilder) iter.Seq2[*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error] { pager := {{ .GophercloudPackage }}.List(c.client, listOpts) return func(yield func(*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error) bool) { - _ = pager.EachPage(ctx, yieldPage({{ .GophercloudPackage }}.Extract{{ .GophercloudType }}s, yield)) + _ = pager.EachPage(ctx, yieldPage({{ .GophercloudPackage }}.Extract{{ .GophercloudType | plural }}, yield)) } } @@ -81,7 +81,7 @@ func New{{ .Kind }}ErrorClient(e error) {{ .Kind }}Client { return {{ .PackageName }}ErrorClient{e} } -func (e {{ .PackageName }}ErrorClient) List{{ .Kind }}s(_ context.Context, _ {{ .GophercloudPackage }}.ListOptsBuilder) iter.Seq2[*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error] { +func (e {{ .PackageName }}ErrorClient) List{{ .Kind | plural }}(_ context.Context, _ {{ .GophercloudPackage }}.ListOptsBuilder) iter.Seq2[*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error] { return func(yield func(*{{ .GophercloudPackage }}.{{ .GophercloudType }}, error) bool) { yield(nil, e.error) } diff --git a/cmd/scaffold-controller/data/controller/actuator.go.template b/cmd/scaffold-controller/data/controller/actuator.go.template index 0d70a10f8..b60af94f2 100644 --- a/cmd/scaffold-controller/data/controller/actuator.go.template +++ b/cmd/scaffold-controller/data/controller/actuator.go.template @@ -130,7 +130,7 @@ func (actuator {{ .PackageName }}Actuator) ListOSResourcesForAdoption(ctx contex // TODO(scaffolding): Add more adoption filters } - return actuator.osClient.List{{ .Kind }}s(ctx, listOpts), true + return actuator.osClient.List{{ .Kind | plural }}(ctx, listOpts), true } func (actuator {{ .PackageName }}Actuator) ListOSResourcesForImport(ctx context.Context, obj orcObjectPT, filter filterT) (iter.Seq2[*osResourceT, error], progress.ReconcileStatus) { @@ -163,7 +163,7 @@ func (actuator {{ .PackageName }}Actuator) ListOSResourcesForImport(ctx context. // TODO(scaffolding): Add more import filters } - return actuator.osClient.List{{ .Kind }}s(ctx, listOpts), {{ if len .ImportDependencies }}reconcileStatus{{ else }}nil{{ end }} + return actuator.osClient.List{{ .Kind | plural }}(ctx, listOpts), {{ if len .ImportDependencies }}reconcileStatus{{ else }}nil{{ end }} } func (actuator {{ .PackageName }}Actuator) CreateResource(ctx context.Context, obj orcObjectPT) (*osResourceT, progress.ReconcileStatus) { diff --git a/cmd/scaffold-controller/data/controller/controller.go.template b/cmd/scaffold-controller/data/controller/controller.go.template index 165f53169..44cb56597 100644 --- a/cmd/scaffold-controller/data/controller/controller.go.template +++ b/cmd/scaffold-controller/data/controller/controller.go.template @@ -41,8 +41,8 @@ import ( const controllerName = "{{ .PackageName }}" -// +kubebuilder:rbac:groups=openstack.k-orc.cloud,resources={{ .PackageName }}s,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=openstack.k-orc.cloud,resources={{ .PackageName }}s/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=openstack.k-orc.cloud,resources={{ .PackageName | plural }},verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=openstack.k-orc.cloud,resources={{ .PackageName | plural }}/status,verbs=get;update;patch type {{ .PackageName }}ReconcilerConstructor struct { scopeFactory scope.Factory diff --git a/cmd/scaffold-controller/data/tests/dependency/README.md.template b/cmd/scaffold-controller/data/tests/dependency/README.md.template index 640cc6ae6..f8840617f 100644 --- a/cmd/scaffold-controller/data/tests/dependency/README.md.template +++ b/cmd/scaffold-controller/data/tests/dependency/README.md.template @@ -2,11 +2,11 @@ ## Step 00 -Create {{ .Kind }}s referencing non-existing resources. Each {{ .Kind }} is dependent on other non-existing resource. Verify that the {{ .Kind }}s are waiting for the needed resources to be created externally. +Create {{ .Kind | plural }} referencing non-existing resources. Each {{ .Kind }} is dependent on other non-existing resource. Verify that the {{ .Kind | plural }} are waiting for the needed resources to be created externally. ## Step 01 -Create the missing dependencies and verify all the {{ .Kind }}s are available. +Create the missing dependencies and verify all the {{ .Kind | plural }} are available. ## Step 02 @@ -14,7 +14,7 @@ Delete all the dependencies and check that ORC prevents deletion since there is ## Step 03 -Delete the {{ .Kind }}s and validate that all resources are gone. +Delete the {{ .Kind | plural }} and validate that all resources are gone. ## Reference diff --git a/cmd/scaffold-controller/data/tests/import-error/README.md.template b/cmd/scaffold-controller/data/tests/import-error/README.md.template index 267054036..5c33286a3 100644 --- a/cmd/scaffold-controller/data/tests/import-error/README.md.template +++ b/cmd/scaffold-controller/data/tests/import-error/README.md.template @@ -2,7 +2,7 @@ ## Step 00 -Create two {{ .Kind }}s with identical specs. +Create two {{ .Kind | plural }} with identical specs. ## Step 01 diff --git a/cmd/scaffold-controller/main.go b/cmd/scaffold-controller/main.go index e26dfe300..6bfd96ae0 100644 --- a/cmd/scaffold-controller/main.go +++ b/cmd/scaffold-controller/main.go @@ -239,6 +239,7 @@ func render(srcDir, distDir string, resource *templateFields) { var funcMap = template.FuncMap{ "lower": strings.ToLower, "camelCase": toCamelCase, + "plural": pluralize, } tpl := template.Must(template.New(tplName).Funcs(funcMap).Parse(string(templateContent))) @@ -298,6 +299,38 @@ func camelToSnake(s string) string { return strings.ToLower(s) } +// pluralize returns the English plural of the given word. +// It handles common suffixes: consonant+y → ies, sibilants → es, +// and falls back to appending s. +func pluralize(s string) string { + if s == "" { + return s + } + + lower := strings.ToLower(s) + + // Words ending in a consonant followed by "y": replace "y" with "ies" + if strings.HasSuffix(lower, "y") { + // Check the character before 'y' is a consonant (not a vowel) + if len(lower) >= 2 { + beforeY := lower[len(lower)-2] + if !strings.ContainsRune("aeiou", rune(beforeY)) { + return s[:len(s)-1] + "ies" + } + } + } + + // Words ending in s, sh, ch, x, z: append "es" + for _, suffix := range []string{"s", "sh", "ch", "x", "z"} { + if strings.HasSuffix(lower, suffix) { + return s + "es" + } + } + + // Default: append "s" + return s + "s" +} + // toCamelCase converts a string to camelCase. // From https://stackoverflow.com/questions/70083837/how-to-convert-a-string-to-camelcase-in-go func toCamelCase(s string) string { diff --git a/cmd/scaffold-controller/main_test.go b/cmd/scaffold-controller/main_test.go new file mode 100644 index 000000000..f5819bf37 --- /dev/null +++ b/cmd/scaffold-controller/main_test.go @@ -0,0 +1,78 @@ +package main + +import "testing" + +func TestPluralize(t *testing.T) { + tests := []struct { + input string + want string + }{ + // Empty string + {"", ""}, + + // Default: append "s" + {"Flavor", "Flavors"}, + {"Network", "Networks"}, + {"Server", "Servers"}, + {"Image", "Images"}, + {"Port", "Ports"}, + {"Router", "Routers"}, + {"Subnet", "Subnets"}, + {"Trunk", "Trunks"}, + {"Volume", "Volumes"}, + {"User", "Users"}, + {"Role", "Roles"}, + {"Domain", "Domains"}, + {"Endpoint", "Endpoints"}, + {"Limit", "Limits"}, + {"Region", "Regions"}, + {"Group", "Groups"}, + {"Service", "Services"}, + {"FloatingIP", "FloatingIPs"}, + {"KeyPair", "KeyPairs"}, + + // Compound names (still just +s) + {"SecurityGroup", "SecurityGroups"}, + {"ServerGroup", "ServerGroups"}, + {"VolumeType", "VolumeTypes"}, + {"ShareNetwork", "ShareNetworks"}, + {"AddressScope", "AddressScopes"}, + {"ApplicationCredential", "ApplicationCredentials"}, + {"RoleAssignment", "RoleAssignments"}, + {"RegisteredLimit", "RegisteredLimits"}, + {"RouterInterface", "RouterInterfaces"}, + + // Consonant + y → ies + {"Policy", "Policies"}, + {"policy", "policies"}, + {"QoSPolicy", "QoSPolicies"}, + {"qospolicy", "qospolicies"}, + + // Vowel + y → just s (not ies) + {"Key", "Keys"}, + {"key", "keys"}, + + // Sibilant endings → es + {"address", "addresses"}, + {"class", "classes"}, + {"bus", "buses"}, + {"match", "matches"}, + {"box", "boxes"}, + {"buzz", "buzzes"}, + {"mesh", "meshes"}, + + // Lowercase defaults + {"flavor", "flavors"}, + {"network", "networks"}, + {"securitygroup", "securitygroups"}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got := pluralize(tt.input) + if got != tt.want { + t.Errorf("pluralize(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +}