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

Adding podOptions to the Tenant specification #737

Closed
prometherion opened this issue Mar 24, 2023 · 3 comments
Closed

Adding podOptions to the Tenant specification #737

prometherion opened this issue Mar 24, 2023 · 3 comments
Labels
needs-discussion No outline on the feature, discussion is welcome
Milestone

Comments

@prometherion
Copy link
Member

The current v1beta2 Tenant API resource has multiple specs that could be added in a single struct.

apiVersion: capsule.clastix.io/v1beta2
kind: Tenant
metadata:
  name: test
spec:
  //  Specifies the allowed RuntimeClasses assigned to the Tenant. Capsule
  // assures that all Pods resources created in the Tenant can use only one of
  // the allowed RuntimeClasses. Optional.
  runtimeClasses:
  // Specifies the allowed priorityClasses assigned to the Tenant. Capsule
  // assures that all Pods resources created in the Tenant can use only one of
  // the allowed PriorityClasses. A default value can be specified, and all the
  // Pod resources created will inherit the declared class. Optional.
  priorityClasses:
  // Specify the allowed values for the imagePullPolicies option in Pod                                                                                                                                            
  // resources. Capsule assures that all Pod resources created in the Tenant can                                                                                                                                   
  // use only one of the allowed policy. Optional.
  imagePullPolicies:
  // Specifies the trusted Image Registries assigned to the Tenant. Capsule                                                                                                                                        
  // assures that all Pods resources created in the Tenant can use only one of                                                                                                                                     
  // the allowed trusted registries. Optional.
  containerRegistries:

The proposal is to group all of these in a new spec parent key, such as podOptions, as we're doing with namespaceOptions, ingressOptions and serviceOptions.

apiVersion: capsule.clastix.io/v1
kind: Tenant
metadata:
  name: test
spec:
  podOptions:
    runtimeClasses:
    priorityClasses:
    imagePullPolicies:
    containerRegistries:

@bsctl @oliverbaehler @MaxFedotov please, share your thoughts!

@prometherion prometherion added the needs-discussion No outline on the feature, discussion is welcome label Mar 24, 2023
@prometherion prometherion added this to the v0.4.0 milestone Mar 24, 2023
@oliverbaehler
Copy link
Collaborator

oliverbaehler commented Apr 9, 2023

@prometherion makes sense to me. But since this is again an API bump i would like to explore more generic options as well.

Moving towards Kyverno Policies

As mentioned in #51, we should reconsider moving the capsule policy engine towards Kyverno policies. Kyverno seems to be the most adapted policy engine by now and we have already seen so many use-cases where it's in use. I am coming up with that idea, since it would change .

This is just a super experimental thought regarding this topic. Just wanted to hear you guys thoughts about it. If it could be remotely interesting we should move it to.a new issues. This would require probably a hefty rewrite, but might give capsule even more framework characteristics + simplifies maintenance.

So just as a quick idea:

apiVersion: capsule.clastix.io/v1
kind: Tenant
metadata:
  name: test
spec:
 policies:
 - policy: "capsule-priorityclasses"
   # Here come configurations for the policy
   config:
     matchLabels:
       env: "production"

Probably we should then work with ownerReferences between policies and tenants to prevent deletion. Also the policy should probably allow referencing of custom policies. But thats already to far for now.

So the capsule controller would ensure that these policies exists (eg. capsule-runtimeclass, capsule-priorityclasses). They are constant. For each tenant referencing such a policy, we add a validating rule:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: capsule-runtimeclass
spec:
  validationFailureAction: Enforce
  rules:

  # Added for tenant "test", updated according to configuration under policies and removed on tenant deletion
  - name: tenant-test-runtimeclass
 
    # Gather allowed RuntimeClasses for tenant
    context:
      - name: runtimeclasses
        apiCall:
          urlPath: "/apis/node.k8s.io/v1/runtimeclasses"
          # This depends on the configuration on the policy (converts the given selector/requirements). A bit hacky.
          jmesPath: "items[?metadata.labels.\"env\" == 'production'].metadata.name | []" 
        
   # Generated by capsule-controller
    match:
     # We can leverage the entire selector system of kyverno (OR, AND operations) (namespaceSelectors) etc. 
      any:
      - resources:
          kinds:
          - Pods
          namespaceSelector:
            matchExpressions:
                - key: capsule.clastix.io/tenant
                  operator: In
                  values: 
                  - test
                  
    # Can also be constant and managed by the capsule-controller
    validate:
      message: "Tenant test enforces these runtimeclasses {{runtimeclasses}}"
        deny: 
          conditions:
            any:
              # Source not in Public Namespaces
              - key: "{{request.object.spec.runtimeClassName}}"
                operator: NotIn
                value: "{{runtimeclasses}}"

As said, just a quick and dirty idea.. :)
So it would probably simplify development of new features, since it wouldn't really require api bumps on the tenants and we could constantly release new policies. However that's just a simple point of view. I guess there would also be a lot of dependency checking etc. to verify everything is running alright.

Some concerns:

  • Version Dependency to Kyverno (or general requirement for Kyverno to be installed and functional). While the second one is always a concern when running Kyverno, the requirement for capsule to have a running Kyverno instance might be controversial.
  • Performance. Don't know how good these policies will perform with large amounts of tenants.

Not sure what you guys think about such a change.

@bsctl
Copy link
Member

bsctl commented Apr 9, 2023

@oliverbaehler this has been discussed many times in #51. However what prevented us to move in that direction was the hard dependency we will introduce. I would suggest two different approaches to explore further:

  • leverage the new native Validating Admission Policies just introduced in 1.26 (beta).
  • use the Capsule Global Tenant Resources to propagate Kyverno policies in addition to the Capsule native policies. In this case there’s still dependency but not mandatory: users are free to use native policies or, if they have already deployed Kyverno, custom Kyverno policies.

@bsctl
Copy link
Member

bsctl commented Oct 16, 2023

@maxgio92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-discussion No outline on the feature, discussion is welcome
Projects
None yet
Development

No branches or pull requests

3 participants