seedwing-io / seedwing-proxy

Policy-enforcing Artifact Proxy
Apache License 2.0
2 stars 3 forks source link

Enforcement configuration #19

Closed jcrossley3 closed 1 year ago

jcrossley3 commented 1 year ago

What's the simplest way to configure policy enforcement in the proxy?

Currently, we have an option, default, with possible values of allow and deny. These determine whether the proxy fails a request (500 status) when the policy engine server is unreachable. But this option has no bearing if the engine is available: the proxy simply relays the status code from the engine: 200 if the policy pattern is matched, and 4xx otherwise.

We'd like the user to be able to configure this behavior, ideally in a single option that encapsulates the current behavior of default described above.

Something along the lines of log levels, perhaps?

policy = ignored       # seems silly?
policy = checked       # engine must be available
policy = matched       # engine must be available and pattern must match
bobmcwhirter commented 1 year ago

I like that, but I'd change "matched" to "enforced" maybe. And "disabled" instead of "ignored"?

policy = [ disabled | checked | enforced ]

lkatalin commented 1 year ago

Sincechecked means the policy engine/server is available but the pattern didn't match, it seems like this is the category that needs to be broken down further to specify the proxy's behavior. Is there any situation in which this scenario should do something other than either warn or fail? Do we want any more granularity, ex. being able to specify "pattern A must match and be enforced, but pattern B can be warn-only"?

In any case I think it is more clear to call "checked" something like "warn", so perhaps policy = [disable | warn | enforce]

jcrossley3 commented 1 year ago

I agree checked ain't great. I was thinking maybe report or log just to better convey (than checked or warn) that the proxy is actually logging the failure into some semblance of a report.

WDYT?

jcrossley3 commented 1 year ago

@lkatalin

Sincechecked means the policy engine/server is available but the pattern didn't match,

Just to clarify: I was thinking checked would mean the engine must be available, effectively the same behavior as what default = deny does currently. Which is "return 500 if the engine isn't reachable".

it seems like this is the category that needs to be broken down further to specify the proxy's behavior. Is there any situation in which this scenario should do something other than either warn or fail? Do we want any more granularity, ex. being able to specify "pattern A must match and be enforced, but pattern B can be warn-only"?

Good questions. I feel like the pattern language itself can support that specification and from the perspective of the proxy, a request either passes or fails, but I don't hold that opinion strongly.

In any case I think it is more clear to call "checked" something like "warn", so perhaps policy = [disable | warn | enforce]

How should the proxy behave if policy = warn and the engine can't be reached? If it doesn't return 500 for any request, then I don't see it as usefully different from policy = disable.

lkatalin commented 1 year ago

@jcrossley3

Just to clarify: I was thinking checked would mean the engine must be available, effectively the same behavior as what default = deny does currently. Which is "return 500 if the engine isn't reachable".

Thanks, yes, your clarification helps. To be specific, how about:

policy = disabled: No policies are checked, the policy server does not need to be reachable. All deps download. Fails when: never. Warns when: never. (This setting does seem kind of silly, but I can also see testing uses for it, like "we just want to make sure non-policy parts of the program are running correctly", or even "I don't want to delete or disable my entire proxy and its configs but don't want to use it on the crates already pointing to it for the next week" or probably others.)

policy = warn (or whatever we want to call this): The policy server must be reachable and will evaluate the request and return the result of the evaluation, but the deps will be downloaded regardless of the outcome of the evaluation. Fails when: policy server is not reachable. Warns when: policy does not match.

policy = enforce: The policy server must be reachable and also the policy pattern must be matched (the policy evaluation must pass) in order to download the deps. Fails when: policy server is not reachable or pattern is not matched. Warns when: never.

Thoughts on the above? Edits welcome, maybe I missed some cases.

I feel like the pattern language itself can support that specification and from the perspective of the proxy, a request either passes or fails, but I don't hold that opinion strongly.

Does the pattern language have a way to say something like "I want to fail if the license is Apache, but warn if the package is not signed" or vice versa? It may be in there already, I'm less familiar with that.

I was thinking maybe report or log just to better convey (than checked or warn) that the proxy is actually logging the failure into some semblance of a report.

I personally think logging setting/level should be a separate concern from enforcement settings because I may want to log even when I'm setting the policy to "enforce", or I may want to warn without logging. For that reason I also think it's more clear to have a policy setting like "warn" instead of "log" to describe the situation where policy is being evaluated but downloads won't fail.

lkatalin commented 1 year ago

Maybe "test" instead of "warn"? Not sure.

jcrossley3 commented 1 year ago

Thoughts on the above? Edits welcome, maybe I missed some cases.

I agree with your descriptions for each level.

Does the pattern language have a way to say something like "I want to fail if the license is Apache, but warn if the package is not signed" or vice versa? It may be in there already, I'm less familiar with that.

I don't think so. I'm still not sure I care about being warned that a policy isn't "quite matched", whatever that might mean. I feel like from the perspective of a build tool, I only care about pass/fail.

I personally think logging setting/level should be a separate concern from enforcement settings because I may want to log even when I'm setting the policy to "enforce", or I may want to warn without logging. For that reason I also think it's more clear to have a policy setting like "warn" instead of "log" to describe the situation where policy is being evaluated but downloads won't fail.

I've been thinking we log everything all the time anyway, and the levels purely determine what responses we give the build tool. But my thinking may need to be expanded. :)

I still don't love warn as the name for that middle level, mostly because to me, it implies a different response to the build tool, which obviously isn't the case. And test is already a pretty overloaded term. How about evaluate? dry-run?

I'm happy to go with warn or whatever you like better for now until a better name reveals itself.

bobmcwhirter commented 1 year ago

Hm, great question. Currently each pattern has no concept of how strongly it should be enforced.

If that is needed at the proxy level, then we might have to go beyond a single flag? But that then becomes kinda fancy interpretation of the rationale, which is... much.

I'll think upon it.

lkatalin commented 1 year ago

I think dry-run is also a good name, or if we are logging all the time anyway then log-only may not be a bad one either.

The more I think about it, the granularity to say "fail on this, but warn on this other thing" would make more sense at the pattern level than in the proxy's policy enforcement, which could just be the 3 simple options.

I do think it could be useful in the pattern, for example one use case might be that org-level policies can be warn-only, but some external authority's policy has to be satisfied or it will fail. Though the downside then is the added complexity of being able to say "warn only" both in the pattern and in the proxy's policy enforcement config.