Added good practices rbac secrets

[enekofb]

We are adding a set of curated policies to help customers protect secrets in the context of pipelines (but eventually applicable to any secret).

Part of weaveworks/weave-gitops#3540

And you could use how it is recommended to be used in this section

[enekofb]

@squaremo we need PolicyConfig to configure these parameters

[squaremo]

Yep. Should it be described in the readme then?

[serboctor]

@enekofb @squaremo The policyconfig is used to override the parameters configured in the policy for certain apps/workspaces/tenants/resources not to configure the policies in general.

[MostafaMegahid]

+1 to @serboctor - I already spoke with @enekofb that we should include any default exclusions in the policy itself.

[enekofb]

@MostafaMegahid @serboctor

please see the usage of the policy and policyConfig here

where policyConfig is being applied in the context of the flux-system app

apiVersion: pac.weave.works/v2beta2
kind: PolicyConfig
metadata:
  name: allow-flux
spec:
  match:
    apps:
      - kind: Kustomization
        name: flux-system
        namespace: flux-system

let me know whether you think is a valid scenario?

I think it matches what was suggested as applies to an app

[enekofb]

I already spoke with @enekofb that we should include any default exclusions in the policy itself.

The couple of issues i could think setting the default exclusions are:

1. In case you as customer does not need the exclusion, would open an attack surface that they will need to manage. It will expose the customer or it will limiting its usability: the customer would prefer not to use it than to manage the risk.

2. In the case that the customer does not have the default value or have multiple values, that policy would be usable

Having this split, would allow a better separation of concerns between

• us = provide the policy so we provide the semantics to enforce + way to exclude
• customer = defines the exclusion context

[serboctor]

please see the usage of the policy and policyConfig here

@enekofb The link is a localhost so I can't check it but I agree entirely that we shouldn't have default exclusions. I misunderstood your initial comment, though it was regarding all parameters. This sounds good to me.

[enekofb]

@serboctor here the right link to the PolicyConfig usage

[MostafaMegahid]

+1 to @serboctor - I already spoke with @enekofb that we should include any default exclusions in the policy itself.

[serboctor]

@enekofb we have a check that updates the CRs with the rego code from the adjacent rego file. And we also usually have tests. I understand that those policies are different because they're basically a specific implementation of policies in the example dir, so may be they can have their own dir that would indicate that to make it less confusing for anyone using this repo?

[enekofb]

@enekofb we have a check that updates the CRs with the rego code from the adjacent rego file. And we also usually have tests. I understand that those policies are different because they're basically a specific implementation of policies in the example dir, so may be they can have their own dir that would indicate that to make it less confusing for anyone using this repo?

@serboctor thanks for your message,

I could also add the tests and the code if you think that would be better (or any other solution that would be more maintainable).

Just let me know what would be your ideal solution and will try to align.

[serboctor]

@serboctor thanks for your message,

I could also add the tests and the code if you think that would be better (or any other solution that would be more maintainable).

Just let me know what would be your ideal solution and will try to align.

@enekofb adding the code and the tests would definitely be ideal 🙏🏼🙏🏼 we have old tests that were using an old approach and new tests. We still havent converted everything. If the policies you référenced from the example are using the old tests method, then you can ignore adding tests for now unless if you're up for writing new ones. @waleedhammam @Samra10 can tell you more about the difference between the old and new tests

[enekofb]

@serboctor thanks for your message,
I could also add the tests and the code if you think that would be better (or any other solution that would be more maintainable).
Just let me know what would be your ideal solution and will try to align.

@enekofb adding the code and the tests would definitely be ideal 🙏🏼🙏🏼 we have old tests that were using an old approach and new tests. We still havent converted everything. If the policies you référenced from the example are using the old tests method, then you can ignore adding tests for now unless if you're up for writing new ones. @waleedhammam @Samra10 can tell you more about the difference between the old and new tests

Done @serboctor feel free to check whether that is closer to what you have in mind. Feel free to share any other thought.

Thanks!

[serboctor]

I am not sure why the original example policies didn't contain a targets field in the yaml, but they should...the agent uses it to match. The check in the rego code will stop the evaluation anyway, but there's no need to reach this point if it can be stopped early on.

[serboctor]

please see the usage of the policy and policyConfig here

@enekofb The link is a localhost so I can't check it but I agree entirely that we shouldn't have default exclusions. I misunderstood your initial comment, though it was regarding all parameters. This sounds good to me.

[enekofb]

I am not sure why the original example policies didn't contain a targets field in the yaml, but they should...the agent uses it to match. The check in the rego code will stop the evaluation anyway, but there's no need to reach this point if it can be stopped early on.

@serboctor added targets to policies and aligned tests. feel free to review and suggest any other thing missing.

[serboctor]

LGTM....thank you for tackling my endless comments!

[serboctor]

LGTM