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

wip - Added validation policies for Deployment and DeploymentConfig #19

Closed
wants to merge 0 commits into from
Closed

wip - Added validation policies for Deployment and DeploymentConfig #19

wants to merge 0 commits into from

Conversation

garethahealy
Copy link
Contributor

What is this PR About?

Adds validation policies suggested by @noelo (https://github.com/noelo/omydb)

This is "an early preview" to get some feedback / allow people to ask questions etc.

Question for the mergers (@tylerauerbeck or @pabrahamsson)

  • Since the PR is quite large. Do you want it split up into several? or happy to merge as a whole?

cc: @redhat-cop/rego-policies

@garethahealy
Copy link
Contributor Author

DONT MERGE

@garethahealy
Copy link
Contributor Author

CC @springdo @ckavili for review/input/thoughts

@springdo
Copy link
Contributor

@garethahealy - now that some of the other PRs are merged in, do you want to split this puppy up into smaller chunks and smash em in?

@garethahealy
Copy link
Contributor Author

@springdo ; whats best/easiest way for you to review? PR per group?

i.e.: split this into 8 PRs?

@garethahealy
Copy link
Contributor Author

garethahealy commented Jun 24, 2020

CI was failing due to required conftest 0.19.0 feature:

# Error: get test result: running warn rules: run multiple queries: run query: evaluating policy: policy/warn-k8s_ocp-deployment_deploymentconfig-bestpractices.rego:176: eval_builtin_error: units.parse_bytes: units.parse_bytes error: byte unit mi not recognized

@springdo
Copy link
Contributor

springdo commented Jun 24, 2020

@garethahealy - few things at a quick glance on this monster pr .....

  • Perhaps for each policy's readability, some docs would be good. Im thinking, where you pulled the policy idea from. So, for example, for the Readiness Probe you could point to something like this https://learnk8s.io/production-best-practices#application-development.
  • The helper functions is getting big, would be good to split them out into some kinda module or dep that could be pulled in by other library. Im thinking, in containers-quickstarts or something similar. I think I'll raise that as a separate issue though as not directly linked by this PR
  • Naming of some of the files could be easier, ie the policy that checks labels bp could say that instead of warn-k8s_ocp-all-bestpractices ie {policy-type}-{platform}-{kind}-{other...} = warn-k8s_ocp-label-bestpractices. I think the policies will be easier to maintain as smaller specific chunks in this way

@redhat-cop-ci-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: garethahealy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants