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

[BUG] Kyverno misinterprets pod spec environment variable placeholders as references #2413

Closed
seh opened this issue Sep 20, 2021 · 4 comments · Fixed by #2433
Closed

[BUG] Kyverno misinterprets pod spec environment variable placeholders as references #2413

seh opened this issue Sep 20, 2021 · 4 comments · Fixed by #2433
Assignees
Labels
bug Something isn't working

Comments

@seh
Copy link
Contributor

seh commented Sep 20, 2021

Software version numbers

  • Kubernetes version: 1.19.7
  • Kubernetes platform: kOps in AWS
  • Kyverno version: 1.4.3

Describe the bug

Per this discussion in the "kyverno" channel in the "Kubernetes" Slack workspace, it is not possible to include environment variable placeholders ($(...)) in pod specs without Kyverno mistaking them for "variable references.".

The regular expression used to detect these references in string fields is lenient. We can tighten it to mandate that relative field paths start with a period or a forward slash:

var RegexReferences = regexp.MustCompile(`\$\((?:<=?|>=?|!)?[^\ )]+\)`)

However, that's more stringent than the the Go path package that Kyverno uses to manipulate these paths. Kyverno uses the path.Join function to join paths that are not absolute—implying that they're relative—which should accommodate any path that does not start with a forward slash character.

Even if we wish to mandate that a relative path must start with a period,

var RegexReferences = regexp.MustCompile(`\$\((?:<=?|>=?|!)?[/.][^\ )]*\)`)

the augmented regular expression above still does not allow Kyverno to ignore environment variable references in pod specs, as those environment variable names can start with a period.

To Reproduce

  1. Write a Kyverno Policy or ClusterPolicy rule targeting a pod spec.
  2. In one of the "spec.containers[].env[].value" fields or one of the "spec.containers[*].args" entries, refer to an environment variable defined nearby in the pod spec, such as $(POD_NAMESPACE).
  3. Attempt to validate the policy using the kyverno validate tool.
  4. Observe that Kyverno complains that variable substitution fails, as it can't find a field named "POD_NAMESPACE."

Expected behavior

There should be some way to write otherwise valid Kubernetes manifests together with Kyverno's various placeholders, retaining the original and intended meaning.

@seh seh added the bug Something isn't working label Sep 20, 2021
@JimBugwadia
Copy link
Member

An option is to add support for escaping the $(...):

apiVersion: kyverno.io/v1
kind: Policy
metadata:
  name: add-otel-resource-env
spec:
  rules:
  - name: imbue-pod-spec
    match:
      resources:
        kinds:
        - v1/Pod
        annotations:
          inject.some.org/open-telemetry-resource: "define"
    mutate:
      patchStrategicMerge:
        spec:
          initContainers: &patch
          - (name): "?*"
            env:
            - name: NODE_NAME
              valueFrom:
                fieldRef:
                  fieldPath: spec.nodeName
            - name: NODE_IP_ADDRESS
              valueFrom:
                fieldRef:
                  fieldPath: status.hostIP
            - name: OTEL_RESOURCE_ATTRIBUTES
              value: >-
                k8s.namespace.name=\$(POD_NAMESPACE),
                k8s.node.name=\$(NODE_NAME),
                k8s.pod.name=\$(POD_NAME),
                k8s.pod.primary_ip_address=\$(POD_IP_ADDRESS),
                k8s.pod.service_account.name=\$(POD_SERVICE_ACCOUNT_NAME),
                k8s.pod.uid=\$(POD_UID),
                net.host.ip=\$(NODE_IP_ADDRESS)

When Kyverno sees as \$(...) it can simply replace it with $(...) and not lookup values from the policy definition.

Would that work?

There is a related issue with {{...}}: #1833 where a similar scheme could be applied.

@JimBugwadia JimBugwadia added this to the Kyverno Release 1.5.0 milestone Sep 20, 2021
@seh
Copy link
Contributor Author

seh commented Sep 20, 2021

Would that work?

Yes, that would suit my needs. In fact, that's how I first wrote my policy, assuming that such an escape syntax would be honored. After that, I tried doubling the dollar signs, to no avail.

There is a related issue with {{...}}: #1833 where a similar scheme could be applied.

Oh, yes, I saw that issue earlier. Reading it again, I now see @rdavyd requesting something similar in #1833 (comment).

@chipzoller
Copy link
Member

So did anyone check whether this escape mechanism works for the {{ }} notation as requested here?

@JimBugwadia
Copy link
Member

No, does not look like escaping variables \{{ }} is handled. There is an open issue for it: #2513.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants