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

FWI-5232 Conditional expressions should be at very top of additionalSchemaStrings #1025

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

jslivka
Copy link
Contributor

@jslivka jslivka commented Jan 3, 2024

This PR fixes #1003

Checklist

  • I have signed the CLA
  • I have updated/added any relevant documentation

Description

Polaris checks utilize additionalSchemaStrings, an additional section of jsonschema that will only evaluate if the object conforms to schemaString. The conditional statements in additionalSchemaStrings need to be arranged in a manner such that when there's nothing to evaluate, the string is empty (not the object therein).

What's the goal of this PR?

Fix false positives reported when running the Polaris CLI for the following checks:

  • clusterrolebindingClusterAdmin
  • rolebindingClusterAdminClusterRole
  • rolebindingClusterRolePodExecAttach
  • rolebindingRolePodExecAttach

What changes did you make?

adjust the ordering of conditional expressions in additionalSchemaStrings:

e.g.

 additionalSchemaStrings:
   rbac.authorization.k8s.io/ClusterRole: |
-    type: object
-    # This schema is validated for all roleBindings, regardless of their roleRef.
     {{ if eq .roleRef.kind "ClusterRole" }}
     {{ if and (not (hasPrefix .metadata.name "system:")) (ne .metadata.name "gce:podsecuritypolicy:calico-sa") }}
+    # This schema is validated for all roleBindings, regardless of their roleRef.
+    type: object

so that there is not an empty object being evaluated, but rather an empty string

I've also added success test cases as a regression check

What alternative solution should we consider, if any?

Copy link

Fairwinds Insights Scan Results

View the Full Report

✅ No new Action Items detected!

@rbren
Copy link
Contributor

rbren commented Jan 3, 2024

For posterity, here's the line that makes this work:

if strings.TrimSpace(templated) == "" {

@jslivka jslivka merged commit c8394bf into master Jan 3, 2024
8 checks passed
@jslivka jslivka deleted the FWI-5232/jbs branch January 3, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with rolebindingClusterAdminClusterRole and rolebindingClusterRolePodExecAttach
2 participants