open-cluster-management-io / policy-generator-plugin

A Kustomize generator plugin to generate Open Cluster Management policies
Apache License 2.0
29 stars 31 forks source link

Flag to disable placement rule generation #86

Closed OozaiPerez closed 1 year ago

OozaiPerez commented 1 year ago

When a placement is not defined in the PolicyGenerator file an empty placement rule is created like so.

spec:
  clusterSelector:
    matchExpressions: []

This placement rule ends up targeting all managed clusters which is not the desired effect in some scenarios.

I propose a flag that can be set in the PolicyGenerator file that disables the placementRule creation.

dhaiducek commented 1 year ago

@OozaiPerez I understand that targeting all clusters may not be desirable, but a Policy without a Placement won't do anything. So I'd argue that if the default to select all clusters isn't desirable, then a Placement should be specified so that doesn't happen. If you're just trying to see what objects get deployed, then the Policy could be disabled. Alternatively, you could use policyDefaults.placement.placementRuleName to specify a nonexistent PlacementRule. Either way, Policies work hand-in-hand with Placement, so I'm not certain that turning off Placement entirely makes sense here. Is there a use case where you'd want to deploy a Policy without a Placement?

OozaiPerez commented 1 year ago

@dhaiducek My clients use case is that we are using policy sets to group our policies together and have a placement rule defined there. I saw we can omit the placement rule creation by defining which policy set we want it added to in the PolicyGenerator file but we would prefer to keep the list of policies defined in our policy set yaml in another location and that would be one file to look at to see what policies are actively being used.

Also we have different sets for different environments that will not exist on each ACM cluster, not sure if ACM will complain when a set defined is not found on a cluster.

OozaiPerez commented 1 year ago

"Alternatively, you could use policyDefaults.placement.placementRuleName to specify a nonexistent PlacementRule"

This could work for us as well, we were also thinking of using a label that does not exist on any of the clusters

dhaiducek commented 1 year ago

Ah, interesting. I hadn't thought of that--defining the Policies and PolicySets separately does make sense for not generating Placements when one is generated for the PolicySet. I think we can add that in--I'll bring it up with our team.

OozaiPerez commented 1 year ago

Great thank you! For now we will use a placement label that does not exist on the managed clusters

ch-stark commented 1 year ago

thanks for bringing this up @OozaiPerez we can fix it at some time, if it gets urgent please let us know

OozaiPerez commented 1 year ago

Thanks for taking care of this so fast!

dhaiducek commented 1 year ago

@OozaiPerez You're welcome! Do you have a need for it to be delivered into ACM 2.7 for GitOps or is it okay to wait until 2.8 for that? We've entered code freeze for 2.7, so @ch-stark would need to provide input if it should go into 2.7.

OozaiPerez commented 1 year ago

It is fine to wait until 2.8. We are using kustomize to run the policy generator tool

dhaiducek commented 1 year ago

Sounds good--thanks!