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

Install Open-policy-agent and create a ingress-conflicts policy. #242

Merged
merged 11 commits into from
Apr 12, 2019

Conversation

vijay-veeranki
Copy link
Contributor

This PR is to install OPA(open-policy-agent) using helm chart and create a ingress-conflicts policy.

This helm chart installs OPA as a Kubernetes admission controller. Using OPA, which can enforce fine-grained invariants over arbitrary resources in Kubernetes cluster.

Created a ConfigMap that contains the main OPA policy and default response. This policy is used as an entry-point for policy evaluations and returns allowed:true if policies are not matched to inbound data.

Created a policy ingress-conflicts that prevents Ingress objects in different namespaces from sharing the same hostname.

Also updated the default values in Helm chart to get the ingress-conflicts policy necessary permissions and resource access.

to deploy opa using default values from helm chart
* comment out line 7-13 to not use default policy
   provided in the bundle

* enabled configmappolicies for namespace opa
  line 72-73

* defined variables for the opa and kube-mgmt image tags
To apply opa policies under resources/opa
This applies a ConfigMap that contains the
main OPA policy and default response.

This policy is used as an entry-point for policy evaluations and returns allowed:true
if policies are not matched to inbound data.
This policy prevents Ingress objects in different
namespaces from sharing the same hostname
ingress-conflicts policy necessary restrictions/permissions

* updated admissionControllerRules:
line 26-29 to restrict the kinds of operations
and resources that are subject to ingress-conflicts policy checks

* mgmt/replicate:
line 79-81 configured to pull resource
 metadata that needed for ingress-conflicts policy

*rbac:
line 113-139 ClusterRole permissions needed for ingress-conflicts policy
@vijay-veeranki
Copy link
Contributor Author

vijay-veeranki commented Apr 8, 2019

This is been applied to test-1.cloud-platform.service.justice.gov.uk cluster

This can be tested by creating the same ingress on a different namespace, use the below for guidance.
https://www.openpolicyagent.org/docs/kubernetes-admission-control.html

Copy link
Contributor

@alkar alkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM!
I've commented on a few things that I feel we might want to adjust.

Additionally, what do we want to do about =, == and :=? Should we convert the policies to use them as in that handy guide or stick with what is in the examples?

- apiGroups:
- ""
resources:
- configmaps
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an additional comment, it's pretty bad the the helm chart will only set up a ClusterRole so we end up having to give it full access to configmaps. Since it will only ever need to parse configmaps from its own namespace, do you think it's worth the effort to extract this and create a Role and RoleBinding externally?

Copy link
Contributor

@alkar alkar Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we can't :(

open-policy-agent/kube-mgmt#11

@vijay-veeranki-moj has tested this and it's the same behaviour on the latest versions

Use := for assignment and == for equality. This makes the policy easier to read and is recommended practice. The
previous policy would fail to run properly in the opa repl.

Additionally, it removes the operation type check (redundant, we want to check both CREATE and UPDATE) which is set at
the ValidatingWebhookConfig level. Also removes the namespace check (which invalidates the check for ingresses within
the same namespace).
@alkar
Copy link
Contributor

alkar commented Apr 11, 2019

@vijay-veeranki-moj I've just pushed two commits with my recommended changes. This should work fine now I but let's get someone else to test as well.

There is no need to override the values from terraform. Removing them makes maintainance easier since we don't need to
remember to update those values and can rely on defaults.
@vijay-veeranki
Copy link
Contributor Author

@vijay-veeranki-moj I've just pushed two commits with my recommended changes. This should work fine now I but let's get someone else to test as well.

@alkar Agree with you

@vijay-veeranki vijay-veeranki merged commit fd7735f into master Apr 12, 2019
@vijay-veeranki vijay-veeranki deleted the open-policy-agent branch April 12, 2019 12:35
@vijay-veeranki vijay-veeranki restored the open-policy-agent branch April 12, 2019 13:18
@vijay-veeranki vijay-veeranki deleted the open-policy-agent branch April 12, 2019 14:22
@vijay-veeranki vijay-veeranki restored the open-policy-agent branch April 12, 2019 14:26
@razvan-moj-zz razvan-moj-zz deleted the open-policy-agent branch April 12, 2019 14:35
@razvan-moj-zz razvan-moj-zz restored the open-policy-agent branch April 12, 2019 14:36
@razvan-moj-zz razvan-moj-zz deleted the open-policy-agent branch April 12, 2019 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants