feat: Expose OPA warnings to ArgoCD UI

[Sergey-Kizimov]

Closes #9256

Description

The PR implement exposing OPA warning to Argo CD UI
The changes depend on gitops-engine PR
Warnings in the ArgoCD UI will look like this:

Probably makes sense to add a warning icon to affected resources, but I need some assistance with this.

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• Optional. My organization is added to USERS.md.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

[soanni86]

any plans to merge/release it in the nearby future ?

[crenshaw-dev]

@soanni86 I'm advocating to get Intuit time for reviewing this by 2.7 RC1.

[crenshaw-dev]

Got it approved, just waiting for the review ticket to make it into one of my sprints.

[ashutosh16]

@Sergey-Kizimov I was trying to validate the changes in my local, however, the manifest still gets synced with a warning. Not sure If my Admission Review spec is correct. Can you provide the opa policy spec for testing?

[crenshaw-dev]

@ashutosh16 do you mean "synced without a warning"? I think the expectation is that the sync happens, you just get a warning in the UI.

[Sergey-Kizimov]

I used the following OPA manifests:
main.rego

import data.kubernetes.admission

main := {
  "apiVersion": "admission.k8s.io/v1",
  "kind": "AdmissionReview",
  "response": response,
}

default uid := ""

uid := input.request.uid

response := {
    "allowed": false,
    "uid": uid,
    "status": {
        "message": reason,
    },
    "warnings": admission.warn,
} {
    reason = concat(", ", admission.deny)
    reason != ""
}

else := {"allowed": true, "uid": uid, "warnings": admission.warn,}

warnings.rego

package kubernetes.admission

pod_templates[p] {
  input.request.kind.kind == "Pod"
  p := input.request.object
}

pod_templates[p] {
  input.request.kind.kind == "Deployment"
  p := input.request.object.spec.template
}

containers[c] {
  c := pod_templates[_].spec.containers[_]
}

containers[c] {
  c := pod_templates[_].spec.initContainers[_]
}

suffixes = {
  "Ki": 1024,
  "Mi": 1024*1024,
  "Gi": 1024*1024*1024,
  "K": 1000,
  "M": 1000*1000,
  "G": 1000*1000*1000
}

convert(str) = result {
  suffixes[suffix] = factor
  endswith(str, suffix)
  num_str := substring(str, 0, count(str) - count(suffix))
  num := to_number(num_str)
  result := num * factor
}

convert(str) = result {
  result := to_number(str)
}

warn[msg] {
  container := containers[_]
  convert(container.resources.requests.memory) != convert(container.resources.limits.memory)
  msg := sprintf("Memory requests and limits do not match on resource %s/%s", [input.request.object.kind, input.request.object.metadata.name])
}

[Sergey-Kizimov]

@ashutosh16 Do you need any help?

[ashutosh16]

@Sergey-Kizimov Sorry for the delayed response, I tried to verify in my local setup but couldn't see the warning, not sure if my configuration is bad. Would you have some time to do the quick zoom, I'm available in CNCF slack with handle @ashutosh

[ashutosh16]

@Sergey-Kizimov I'm able to verify the changes in my local. however, I would need some more explanation on the feature.

• When the spec contains separate values for limit and resource memory. I see the Sync Result warning, which appears to be reasonable. When I synced again, the warning regarding the sync result was lost and the sync result was green. Since the limit and resource values are still not matching, this behavior is misleading and ArgoCD should display the warning.
  I did not see the AdmissionReview response containing the warning message when I synced the resource a second time. This may be the reason why the sync went ahead without warning.

opa.mov

I'd suggest we should persist the sync result in the App condition or display it as a warning icon on the resource in Tree View.

[crenshaw-dev]

I'd suggest we should persist the sync result in the App condition or display it as a warning icon on the resource in Tree View.

I agree that it's weird for the warning to apply on only one sync. But how would we know whether the warning still applies for subsequent syncs?

[ashutosh16]

I agree that it's weird for the warning to apply on only one sync. But how would we know whether the warning still applies for subsequent syncs?

In this scenario, Argocd's warning message depends on Kubernetes reporting the message back to Argocd. It does not fit in this case because subsequent sync actions do not yield warning messages to argocd/kubectl. I'm not certain if this is an implicit behavior of the opa/validation webhook. There is no issue from argocd side, it displays the message that it received. However, In my opinion, the user experience is confusing when the issue still exists in the manifest and the next sync is applied without a warning message. Is there a way to keep the existing warning?

❯ k apply -f deploy.yaml -n test
Warning: Memory requests and limits do not match on resource Deployment/kustomize-guestbook-ui
deployment.apps/kustomize-guestbook-ui created
❯ k apply -f deploy.yaml -n test
deployment.apps/kustomize-guestbook-ui unchanged

[crenshaw-dev]

I think it’s easy to store the warning message, I’m just not sure how to clear it when it’s no longer relevant, because Argo CD has no insight into whether the warning is still relevant (as far as I know).

[Sergey-Kizimov]

This is the expected behavior for the OPA, the kube-api server does not send a request to the OPA if the resource is not changed.
Yes, it may be tricky to clear the warning, we can analyze the returned message, for example, if apply operation result contains unchanged in the message the warning for the resource is still relevant, but I don't like this solution.

[ashutosh16]

Here is the next step, we can do to get this PR merge.

The PR warns the end-user when OPA waning is generated however it gets confusing when the resource is not modified and the no-ops sync warning is lost. I think It'd be better to log the warning in the audit log in case users want to audit and track the changes.

• Created the issue to log the warning in audit log. @Sergey-Kizimov Would you be able to contribute to this PR or separate PR?

• @crenshaw-dev If no objection on gitops PR, can it be merge?

• @Sergey-Kizimov Once gitops PR gets merged, update go.mod for gitops dependency.

[Sergey-Kizimov]

@ashutosh16, thank you, I can do that

[crenshaw-dev]

@Sergey-Kizimov I had one concern on the gitops-engine PR. Otherwise lgtm!

[zeusal]

What is the situation? I hope this feature will be here soon.