Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make test failing if --plural flag is provided as capitalized #1969

Closed
prafull01 opened this issue Jan 23, 2021 · 4 comments · Fixed by #1970
Closed

make test failing if --plural flag is provided as capitalized #1969

prafull01 opened this issue Jan 23, 2021 · 4 comments · Fixed by #1970
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@prafull01
Copy link
Contributor

prafull01 commented Jan 23, 2021

What did you do:

Create a api with --plural flag and provide it as capitalized, then it breaks the make test all as the same capitalized plural flag goes into the CRD.

prafull@EMPID18004:~/go/src/github.com/prafull01/kubebuilder/testproj$ ../bin/kubebuilder create api --group=praf --kind=Fernandez --version=v1 --plural=Fernandezes
Create Resource [y/n]
y
Create Controller [y/n]
y
Writing scaffold for you to edit...
api/v1/fernandez_types.go
controllers/fernandez_controller.go
Running make:
$ make
/home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
go fmt ./...
go vet ./...
go build -o bin/manager main.go
prafull@EMPID18004:~/go/src/github.com/prafull01/kubebuilder/testproj$ vi PROJECT 
prafull@EMPID18004:~/go/src/github.com/prafull01/kubebuilder/testproj$ make test all
/home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
go fmt ./...
go vet ./...
/home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/bin/controller-gen "crd:trivialVersions=true,preserveUnknownFields=false" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
mkdir -p /home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/testbin
test -f /home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/testbin/setup-envtest.sh || curl -sSLo /home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/testbin/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.7.0/hack/setup-envtest.sh
source /home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/testbin/setup-envtest.sh; fetch_envtest_tools /home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/testbin; setup_envtest_env /home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/testbin; go test ./... -coverprofile cover.out
/bin/sh: 1: source: not found
/bin/sh: 1: fetch_envtest_tools: not found
/bin/sh: 1: setup_envtest_env: not found
vi con?   	sigs.k8s.io/kubebuilder/v3	[no test files]
?   	sigs.k8s.io/kubebuilder/v3/api/v1	[no test files]
fi	Running Suite: Controller Suite
===============================
Random Seed: 1611401178
Will run 0 of 0 specs

STEP: bootstrapping test environment
2021-01-23T16:56:18.280+0530	DEBUG	controller-runtime.test-env	starting control plane	{"api server flags": []}
2021-01-23T16:56:23.879+0530	DEBUG	controller-runtime.test-env	installing CRDs
2021-01-23T16:56:23.879+0530	DEBUG	controller-runtime.test-env	reading CRDs from path	{"path": "../config/crd/bases"}
2021-01-23T16:56:23.881+0530	DEBUG	controller-runtime.test-env	read CRDs from file	{"file": "praf.my.domain_Fernandezes.yaml"}
2021-01-23T16:56:23.902+0530	DEBUG	controller-runtime.test-env	installing CRD	{"crd": "Fernandezes.praf.my.domain"}
Failure [5.635 seconds]
[BeforeSuite] BeforeSuite 
/home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/controllers/suite_test.go:52

  Unexpected error:
      <*errors.StatusError | 0xc0006445a0>: {
          ErrStatus: {
              TypeMeta: {Kind: "", APIVersion: ""},
              ListMeta: {
                  SelfLink: "",
                  ResourceVersion: "",
                  Continue: "",
                  RemainingItemCount: nil,
              },
              Status: "Failure",
              Message: "CustomResourceDefinition.apiextensions.k8s.io \"Fernandezes.praf.my.domain\" is invalid: [metadata.name: Invalid value: \"Fernandezes.praf.my.domain\": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'), spec.names.plural: Invalid value: \"Fernandezes\": a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')]",
              Reason: "Invalid",
              Details: {
                  Name: "Fernandezes.praf.my.domain",
                  Group: "apiextensions.k8s.io",
                  Kind: "CustomResourceDefinition",
                  UID: "",
                  Causes: [
                      {
                          Type: "FieldValueInvalid",
                          Message: "Invalid value: \"Fernandezes.praf.my.domain\": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
                          Field: "metadata.name",
                      },
                      {
                          Type: "FieldValueInvalid",
                          Message: "Invalid value: \"Fernandezes\": a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')",
                          Field: "spec.names.plural",
                      },
                  ],
                  RetryAfterSeconds: 0,
              },
              Code: 422,
          },
      }
      CustomResourceDefinition.apiextensions.k8s.io "Fernandezes.praf.my.domain" is invalid: [metadata.name: Invalid value: "Fernandezes.praf.my.domain": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'), spec.names.plural: Invalid value: "Fernandezes": a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')]
  occurred

  /home/prafull/go/src/github.com/prafull01/kubebuilder/testproj/controllers/suite_test.go:62
------------------------------


Ran 0 of 0 Specs in 5.668 seconds
FAIL! -- 0 Passed | 0 Failed | 0 Pending | 0 Skipped
--- FAIL: TestAPIs (5.67s)
FAIL
coverage: 0.0% of statements
FAIL	sigs.k8s.io/kubebuilder/v3/controllers	5.681s
FAIL
make: *** [Makefile:37: test] Error 1

We should lowercase the --plural flag value if provided in capitalized by user.

/kind bug

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 23, 2021
@prafull01 prafull01 changed the title make test failing if --plural flag is provided as camelCase make test failing if --plural flag is provided as capitalized Jan 23, 2021
@Adirio
Copy link
Contributor

Adirio commented Jan 23, 2021

We have two options here. Either validate the flag to ensure that it is lowercase, or lowercase it ourselves.

If it was the first flag we validate I would probably go with lowercase it ourselves and print a warning to the user, but the approach followed until now is the other one, validate that the user inputed a lowercased string.

@prafull01
Copy link
Contributor Author

IMO, we should lower it ourselves because the input of --kind is Capitalized like Admiral, so in most probable human scenario is user is going to provide the --plural flag as capitalized also like Admirales. This means throwing the error in most of the cases and ask user to provide it as lowercase value.

@Adirio
Copy link
Contributor

Adirio commented Jan 23, 2021

That has a really simple solution, press up arrow, remove the capital, press enter. Same happens if you provide --kind admiral, it will error out. Kept the same behavior in my PR.

@camilamacedo86
Copy link
Member

Hi @prafull01,

the #1970 makes sense for me. Since it is the small fix we can get it merged. However, please feel free to re-open this one or raise a new one and any PR with your suggestions to improve/fix it as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants