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

[epic] Ensure no two ClusterExtensions can manage the same underlying object #736

Open
joelanford opened this issue Apr 4, 2024 · 1 comment
Labels
epic v1.0 Issues related to the initial stable release of OLMv1

Comments

@joelanford
Copy link
Member

No description provided.

@joelanford joelanford self-assigned this Apr 4, 2024
@joelanford joelanford added epic priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. v1.0 Issues related to the initial stable release of OLMv1 and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Apr 4, 2024
@joelanford joelanford removed their assignment Apr 4, 2024
@joelanford
Copy link
Member Author

I spent a little time hacking on this in the Rukpak repo and came up with:

---
apiVersion: admissionregistration.k8s.io/v1beta1
kind: ValidatingAdmissionPolicy
metadata:
  name: single-owner-policy
spec:
  failurePolicy: Fail
  matchConstraints:
    objectSelector:
      matchExpressions:
      - key: "core.rukpak.io/owner-name"
        operator: Exists
    resourceRules:
    - apiGroups:
      - "*"
      apiVersions:
      - "*"
      resources:
      - "*"
      operations:
      - UPDATE

  validations:
  - expression: |
      !("core.rukpak.io/owner-name" in oldObject.metadata.labels) ||
      !("core.rukpak.io/owner-name" in object.metadata.labels) ||
      oldObject.metadata.labels["core.rukpak.io/owner-name"] == object.metadata.labels["core.rukpak.io/owner-name"]
    messageExpression: '"object is owned by BundleDeployment \"" + oldObject.metadata.labels["core.rukpak.io/owner-name"]
      + "\": cannot be adopted by BundleDeployment \"" + object.metadata.labels["core.rukpak.io/owner-name"]
      + "\""'
    reason: Invalid
---
apiVersion: admissionregistration.k8s.io/v1beta1
kind: ValidatingAdmissionPolicyBinding
metadata:
  name: single-owner-policy
spec:
  policyName: single-owner-policy
  validationActions:
  - Deny

It will need to be adapted to change "core.rukpak.io/owner-name" to whatever label we decide to inject from the ClusterExtension API, but other than that my manual tests seemed to do the right thing.

I noticed that testing this in "real" scenarios may be difficult because Helm has its own logic to implement a single-owner policy. Perhaps we could write a test that tries to make concurrent changes to see if we can make if flaky with respect to the race conditions we expect, and then add this policy and check that the flakes disappear.

In addition, I think we can also just have some direct envtest-based tests that go around helm and try to make label changes directly so that we mimic helm but avoid its single-owner logic. That way, we can implement assertion logic based on what we expect this policy to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic v1.0 Issues related to the initial stable release of OLMv1
Projects
Status: No status
Development

No branches or pull requests

1 participant