Policies #367

ChiaraOggeri · 2020-12-07T22:08:13Z

Description

Created a policies folder that contains a OPA policy to verify the correct creation (or update) of a new instance.
In particular it checks if the instance referres to an existing template in the correct namespace.
Fixes # (issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.

Test A
Test B

kingmakerbot · 2020-12-07T22:08:32Z

Hi @ChiaraOggeri. Thanks for your PR.

I am @kingmakerbot.
You can interact with me issuing a slash command in the first line of a comment.
Currently, I understand the following commands:

/rebase: Rebase this PR onto the master branch
/merge: Merge this PR into the master branch
/deploy-staging: Deploy a staging environment to test this PR
/undeploy-staging: Manually undeploy the staging environment

Make sure this PR appears in the CrownLabs changelog, adding one of the following labels:

kind/breaking: 💥 Breaking Change
kind/feature: 🚀 New Feature
kind/bug: 🐛 Bug Fix
kind/cleanup: 🧹 Code Refactoring
kind/docs: 📝 Documentation

giorio94

First of all, thanks @ChiaraOggeri and @sofymunari for this. Looks very interesting to me.

A couple of generic comments (don't know if this PR is already ready for review or not):

Please, use opa fmt to format the rego files.
The rego policy is replicated both in its own file (for testing) and in the gatekeeper constraint. This will soon lead to inconsistencies, since one file is updated and the other not. I would suggest trying to use konstraint to generate the gatekeeper constraints from the base rego policies.

policies/verify-instances-templates-reference/test/rego-rule.rego

giorio94 · 2020-12-15T08:35:50Z

policies/verify-instances-templates-reference/constraintTemplate.yaml

+apiVersion: templates.gatekeeper.sh/v1beta1
+kind: ConstraintTemplate
+metadata: 
+  name: crownlabsconstraint


Suggested change

name: crownlabsconstraint

name: crownlabs-instance-template-reference

I would suggest the same for the CRD name (in the correct case)

policies/verify-instances-templates-reference/README.md

giorio94

Looks in a pretty good shape. A couple of minor additional comments inline.

I just have some concerns about the new policy, but I still have to understand which would be the best solution from the design point of view. We can discuss about that on slack

giorio94 · 2020-12-17T08:22:40Z

.github/workflows/test.yml

@@ -1,6 +1,6 @@
 name: Code testing
 on:
-  pull_request_target:
+  pull_request:


Just to avoid forgetting, this has to be reverted before merging

policies/verify-instances-templates-reference/README.md

giorio94 · 2020-12-22T10:43:39Z

policies/verify-instances-templates-reference/policies/rego_rule.rego

+
+# @title crownlabs verify instance template reference
+
+package crownlabsinstancetemplatereference


Suggested change

package crownlabsinstancetemplatereference

package crownlabs_instance_template_reference

I feel it clearer, in case this modification is possible

giorio94 · 2020-12-22T10:45:51Z

policies/verify-tenant-patch/policies/rego_rule.rego

+
+# @title crownlabs verify tenant patch
+
+package crownlabstenantconstraint


Suggested change

package crownlabstenantconstraint

package crownlabs_tenant_constraint

As before

giorio94 · 2020-12-22T10:47:22Z

policies/verify-instances-templates-reference/policies/rego_rule.rego

+	msg := sprintf("Namespace %v doesn't contain any template", [ns])
+}
+
+violation[{"msg": msg, "details": {"missing_tempalte": [missing]}}] {


Suggested change

violation[{"msg": msg, "details": {"missing_tempalte": [missing]}}] {

violation[{"msg": msg, "details": {"missing_template": [missing]}}] {

giorio94 · 2020-12-22T11:03:52Z

policies/verify-tenant-patch/policies/rego_rule_test.rego

+				"managedFields": [
+					{
+						"operation": "Update",
+						"time": "2020-12-20T11:14:59Z",
+						"apiVersion": "crownlabs.polito.it/v1alpha1",
+						"fieldsType": "FieldsV1",
+						"fieldsV1": {"f:spec": {".": {}, "f:ID": {}, "f:createSandbox": {}, "f:name": {}, "f:publicKeys": {}, "f:surname": {}, "f:workspaces": {}}}, "manager": "kubectl-create",
+					},
+					{
+						"fieldsType": "FieldsV1", "fieldsV1": {"f:spec": {"f:email": {}}},
+						"manager": "kubectl-patch",
+						"operation": "Update",
+						"time": "2020-12-20T11:30:12Z",
+						"apiVersion": "crownlabs.polito.it/v1alpha1",
+					},
+				],


You can drop the managedFields, to shorten this file. The same for all occurrences

giorio94 · 2020-12-22T11:08:03Z

policies/verify-instances-templates-reference/README.md

+## TESTS
+Tests are available in folder [policies](./policies).
+
+## HOW TO RUN


Suggested change

## HOW TO RUN

## HOW TO DEPLOY

Co-authored-by: Sofia Munari <sofymunari@gmail.com>

giorio94 · 2020-12-23T19:46:07Z

/merge

kingmakerbot · 2020-12-23T19:47:59Z

Your staging environment has been correctly teared-down!

pull-request-size bot added the size/L label Dec 7, 2020

kingmakerbot added the sig/operations label Dec 7, 2020

ChiaraOggeri force-pushed the policies branch from 1d8eaba to 1e872e0 Compare December 7, 2020 22:32

pull-request-size bot added size/M and removed size/L labels Dec 7, 2020

ChiaraOggeri force-pushed the policies branch from 871966e to 4c46914 Compare December 11, 2020 23:25

pull-request-size bot added size/L and removed size/M labels Dec 14, 2020

ChiaraOggeri force-pushed the policies branch from 54df605 to d34a9dd Compare December 14, 2020 22:30

giorio94 added the kind/feature New feature or request label Dec 15, 2020

giorio94 reviewed Dec 15, 2020

View reviewed changes

ChiaraOggeri force-pushed the policies branch from 337814e to 1a50b4f Compare December 15, 2020 11:31

kingmakerbot added the sig/devops label Dec 16, 2020

ChiaraOggeri force-pushed the policies branch 5 times, most recently from 0a22870 to 7e2d3aa Compare December 20, 2020 08:27

palexster reviewed Dec 21, 2020

View reviewed changes

policies/verify-instances-templates-reference/README.md Outdated Show resolved Hide resolved

ChiaraOggeri force-pushed the policies branch from 8f2ef7d to 5d8afe7 Compare December 22, 2020 07:22

pull-request-size bot added size/XL and removed size/L labels Dec 22, 2020

ChiaraOggeri force-pushed the policies branch from 58c94ad to a4043ba Compare December 22, 2020 08:35

giorio94 reviewed Dec 22, 2020

View reviewed changes

ChiaraOggeri force-pushed the policies branch 3 times, most recently from fc29a21 to a0d2eb4 Compare December 23, 2020 17:54

pull-request-size bot added size/L and removed size/XL labels Dec 23, 2020

Create OPA policies and tests run on pipeline

f0c2875

Co-authored-by: Sofia Munari <sofymunari@gmail.com>

ChiaraOggeri force-pushed the policies branch from 13624dc to f0c2875 Compare December 23, 2020 18:15

ChiaraOggeri marked this pull request as ready for review December 23, 2020 18:18

palexster approved these changes Dec 23, 2020

View reviewed changes

giorio94 approved these changes Dec 23, 2020

View reviewed changes

kingmakerbot added the merge-requested label Dec 23, 2020

kingmakerbot merged commit b389156 into netgroup-polito:master Dec 23, 2020

kingmakerbot removed the merge-requested label Dec 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Policies #367

Policies #367

ChiaraOggeri commented Dec 7, 2020

kingmakerbot commented Dec 7, 2020

giorio94 left a comment

giorio94 Dec 15, 2020

giorio94 left a comment

giorio94 Dec 17, 2020

giorio94 Dec 22, 2020

giorio94 Dec 22, 2020

giorio94 Dec 22, 2020

giorio94 Dec 22, 2020

giorio94 Dec 22, 2020

giorio94 commented Dec 23, 2020

kingmakerbot commented Dec 23, 2020

	name: crownlabsconstraint
	name: crownlabs-instance-template-reference


		# @title crownlabs verify instance template reference

		package crownlabsinstancetemplatereference

	package crownlabsinstancetemplatereference
	package crownlabs_instance_template_reference


		# @title crownlabs verify tenant patch

		package crownlabstenantconstraint

	package crownlabstenantconstraint
	package crownlabs_tenant_constraint

	violation[{"msg": msg, "details": {"missing_tempalte": [missing]}}] {
	violation[{"msg": msg, "details": {"missing_template": [missing]}}] {

Policies #367

Policies #367

Conversation

ChiaraOggeri commented Dec 7, 2020

Description

How Has This Been Tested?

kingmakerbot commented Dec 7, 2020

giorio94 left a comment

Choose a reason for hiding this comment

giorio94 Dec 15, 2020

Choose a reason for hiding this comment

giorio94 left a comment

Choose a reason for hiding this comment

giorio94 Dec 17, 2020

Choose a reason for hiding this comment

giorio94 Dec 22, 2020

Choose a reason for hiding this comment

giorio94 Dec 22, 2020

Choose a reason for hiding this comment

giorio94 Dec 22, 2020

Choose a reason for hiding this comment

giorio94 Dec 22, 2020

Choose a reason for hiding this comment

giorio94 Dec 22, 2020

Choose a reason for hiding this comment

giorio94 commented Dec 23, 2020

kingmakerbot commented Dec 23, 2020