Refactor `settings::RuleStrategy.rule` as enum

viccuad commented 2 years ago

Is your feature request related to a problem?

Currently, settings::RuleStrategy.rule field is a String.

This means that when deserializing the settings Yaml of a policy, it can deserialize incorrect settings and store them as strings, such as the following incorrect settings:

rule: RunAsAny
ranges:
  - min: 1000
    max: 2000

This will later correctly fail, as validate() is called on the settings, and validate() will return an error.

We could instead fail earlier, on deserialization.

Acceptance criteria

Simplify the code by refactoring settings::RuleStrategy.rule field from String into an enum variable, where the MustRunAs variant is the one containing the list of ranges.

This allows us to potentially simplify the validate() function.

Refactor the present tests as needed.

Alternatives you've considered

No response

Anything else?

See https://github.com/kubewarden/user-group-psp-policy/pull/15.

geeksambhu commented 1 year ago

I am working on this

viccuad commented 1 year ago

After consideration and seeing the work done for https://github.com/kubewarden/user-group-psp-policy/pull/56, I think that doing the checks via the deserializer makes everything needlessly complicated. The card did indeed ask to try to do it via de Deserializer, which I know think it's a wrong approach. I'm sorry for that.

I would be happy just having

pub(crate) struct RuleStrategy {
    pub rule: Rule,
    pub ranges: Vec<IDRange>,
    pub overwrite: bool,
}

Where Rule is an enum with variants MustRunAs, RunAsAny, MustRunAsNonRoot, MayRunAs (with the correct serde attributes for it to deserialize correctly). And having the validation of the settings being performed on runtime at validate().

kubewarden / user-group-psp-policy