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

Attempt to create ClusterRbacAssessmentReport with invalid name #343

Closed
erikgb opened this issue Jul 17, 2022 · 10 comments · Fixed by #1322
Closed

Attempt to create ClusterRbacAssessmentReport with invalid name #343

erikgb opened this issue Jul 17, 2022 · 10 comments · Fixed by #1322
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. target/kubernetes Issues relating to kubernetes cluster scanning

Comments

@erikgb
Copy link
Contributor

erikgb commented Jul 17, 2022

What steps did you take and what happened:

I just booted up a fresh cluster with trivy-operator and some other stuff installed. It seems like our ClusterRbacAssessmentReport-reconciler is attempting to create a report with an invalid name:

{"level":"error","ts":1658084310.4691162,"msg":"Reconciler error","controller":"clusterrole","controllerGroup":"rbac.authorization.k8s.io","controllerKind":"ClusterRole","clusterRole":{"name":"cluster-crd-clusterRole"},"namespace":"","name":"cluster-crd-clusterRole","reconcileID":"a663541b-97ff-4c86-99c2-bdf2fbe7571e","error":"ClusterRbacAssessmentReport.aquasecurity.github.io \"clusterrole-cluster-crd-clusterRole\" is invalid: metadata.name: Invalid value: \"clusterrole-cluster-crd-clusterRole\": a lowercase RFC 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])?)*')","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.12.3/pkg/internal/controller/controller.go:273\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.12.3/pkg/internal/controller/controller.go:234"}

What did you expect to happen:

Resources scanned without errors.

Anything else you would like to add:

[Miscellaneous information that will assist in solving the issue.]

Environment:

  • Trivy-Operator version (use trivy-operator version): 0.1.3
  • Kubernetes version (use kubectl version): RKE2 1.23.8
  • OS (macOS 10.15, Windows 10, Ubuntu 19.10 etc): RHEL 8 on K8s host
@erikgb erikgb added the kind/bug Categorizes issue or PR as related to a bug. label Jul 17, 2022
@jrhunger
Copy link
Contributor

Looks like this happens for any Role or ClusterRole that has capital letters in its name. Seems the name of the CRD is subject to one of the DNS-like object name rules (https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names) however it is basing its name on the Role/ClusterRole it describes, and those objects are not subject to the same rules.

@erikgb
Copy link
Contributor Author

erikgb commented Jul 21, 2022

@jrhunger Thanks, would you know if the name rules are documented somewhere? I am bit surprised to see a clusterrole name with uppercase letters, but I am developer and not a cluster-admin. 😸

Found a similar issue, and suggest we add some logic to our custom resource names. WDYT, @chen-keinan?

@chen-keinan
Copy link
Contributor

chen-keinan commented Jul 21, 2022

@erikgb strange , trivy-operator validate the name and change it if it does not meet the requirements.

@erikgb
Copy link
Contributor Author

erikgb commented Jul 21, 2022

@erikgb strange , trivy-operator validate the name and change it if it does not meet the requirements.

@chen-keinan I think we use the wrong func to validate. Rules for names are stricter than for labels. There seems to be other validation funcs in the same package that we should use to validate resource names.

Ref. https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names

@erikgb
Copy link
Contributor Author

erikgb commented Jul 21, 2022

I think the correct func to use for resource name is validation.IsDNS1123Subdomain. I see that this could be fixed in a couple of places. Not just for the ClusterRbacAssessmentReport.

@chen-keinan
Copy link
Contributor

since the role name is kept in label we have to pass it via label validator as well.

@erikgb
Copy link
Contributor Author

erikgb commented Jul 21, 2022

since the role name is kept in label we have to pass it via label validator as well.

But resource names have stricter validation than labels. So all valid resource names are valid labels.

@erikgb
Copy link
Contributor Author

erikgb commented Jul 21, 2022

Some insights to this problem by Master Liggitt on Slack: https://kubernetes.slack.com/archives/C0EG7JC6T/p1658391621520889

@chen-keinan chen-keinan added target/kubernetes Issues relating to kubernetes cluster scanning priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jul 21, 2022
@jrhunger
Copy link
Contributor

jrhunger commented Dec 1, 2022

@erikgb just curious, did #388 fix this for you?
I am still seeing the issue with v0.7.0, for example:

{"level":"error","ts":1669913117.0058787,"msg":"Reconciler error","controller":"role","controllerGroup":"rbac.authorization.k8s.io","controllerKind":"Role","Role":{"name":"volumesnapshot-admin-Jonathan.Hunsberger","namespace":"ciok-test"},"namespace":"ciok-test","name":"volumesnapshot-admin-Jonathan.Hunsberger","reconcileID":"4a928abd-c766-4524-b3f8-1c0a4309630d","error":"RbacAssessmentReport.aquasecurity.github.io \"role-volumesnapshot-admin-Jonathan.Hunsberger\" is invalid: metadata.name: Invalid value: \"role-volumesnapshot-admin-Jonathan.Hunsberger\": a lowercase RFC 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])?)*')","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:326\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:273\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:234"}

@erikgb
Copy link
Contributor Author

erikgb commented Dec 1, 2022

@jrhunger We have abandoned trivy-operator, and #388 was never reviewed/merged by the single maintainer. So I just closed it when we left. Feel free to pick it up, if you want. 👍

deven0t pushed a commit to deven0t/trivy-operator that referenced this issue Jun 23, 2023
When rbac resources is having capital letter in its name then trivy-operator throws error for creating its configaudit report OR rbac report.
This happens bcoz capital letter name is not allowed in majority of the resources except rbac.
To resolve this, we will make sure that trivy-operator doesnot try create report with capital letter, if capital letter comes then we generate report with hashvalue name
- replace IsValidLabelValue to IsDNS1123Label which allows only lower case value other similar constraint on length

resolve: aquasecurity#343
deven0t pushed a commit to deven0t/trivy-operator that referenced this issue Jun 23, 2023
When rbac resources is having capital letter in its name then trivy-operator throws error for creating its configaudit report OR rbac report.
This happens bcoz capital letter name is not allowed in majority of the resources except rbac.
To resolve this, we will make sure that trivy-operator doesnot try create report with capital letter, if capital letter comes then we generate report with hashvalue name
- replace IsValidLabelValue to IsDNS1123Label which allows only lower case value other similar constraint on length

resolve: aquasecurity#343
deven0t added a commit to deven0t/trivy-operator that referenced this issue Jun 23, 2023
When rbac resources is having capital letter in its name then trivy-operator throws error for creating its configaudit report OR rbac report.
This happens bcoz capital letter name is not allowed in majority of the resources except rbac.
To resolve this, we will make sure that trivy-operator doesnot try create report with capital letter, if capital letter comes then we generate report with hashvalue name
- replace IsValidLabelValue to IsDNS1123Label which allows only lower case value other similar constraint on length

resolve: aquasecurity#343
chen-keinan pushed a commit that referenced this issue Jun 25, 2023
When rbac resources is having capital letter in its name then trivy-operator throws error for creating its configaudit report OR rbac report.
This happens bcoz capital letter name is not allowed in majority of the resources except rbac.
To resolve this, we will make sure that trivy-operator doesnot try create report with capital letter, if capital letter comes then we generate report with hashvalue name
- replace IsValidLabelValue to IsDNS1123Label which allows only lower case value other similar constraint on length

resolve: #343
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. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. target/kubernetes Issues relating to kubernetes cluster scanning
Projects
None yet
3 participants