Skip to content

api-gateway: allow controller to bind PodSecurityPolicy to ServiceAccounts that it creates #1672

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.

Sign up for GitHub

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

Merged
nathancoleman merged 6 commits into from
Nov 4, 2022
Merged

api-gateway: allow controller to bind PodSecurityPolicy to ServiceAccounts that it creates #1672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dc422f6
Add PodSecurityPolicy for all Gateway Deployments
nathancoleman Nov 1, 2022
706fa94
Allow API gateway controller to manage roles + bindings
nathancoleman Nov 1, 2022
71045d9
Add entry to CHANGELOG
nathancoleman Nov 2, 2022
d247baa
Consolidate controller ClusterRole mods for enablePodSecurityPolicies
nathancoleman Nov 3, 2022
860fc31
Update/add unit test coverage for controller ClusterRole
nathancoleman Nov 3, 2022
d92bc7e
Check for additional verbs on cluster role
nathancoleman Nov 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ IMPROVEMENTS:
* Add `tolerations` and `nodeSelector` to Server ACL init jobs and `nodeSelector` to Webhook cert manager. [[GH-1581](https://github.com/hashicorp/consul-k8s/pull/1581)]
* API Gateway: Add `tolerations` to `apiGateway.managedGatewayClass` and `apiGateway.controller` [[GH-1650](https://github.com/hashicorp/consul-k8s/pull/1650)]
* API Gateway: Create PodSecurityPolicy for controller when `global.enablePodSecurityPolicies=true`. [[GH-1656](https://github.com/hashicorp/consul-k8s/pull/1656)]

* API Gateway: Create PodSecurityPolicy and allow controller to bind it to ServiceAccounts that it creates for Gateway Deployments when `global.enablePodSecurityPolicies=true`. [[GH-1672](https://github.com/hashicorp/consul-k8s/pull/1672)]

## 1.0.0-beta4 (October 28, 2022)

Expand Down
20 changes: 15 additions & 5 deletions charts/consul/templates/api-gateway-controller-clusterrole.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,21 @@ rules:
- patch
- update
{{- if .Values.global.enablePodSecurityPolicies }}
- apiGroups: ["policy"]
resources: ["podsecuritypolicies"]
resourceNames:
- {{ template "consul.fullname" . }}-api-gateway-controller
- apiGroups:
- policy
resources:
- podsecuritypolicies
verbs:
- use
Comment on lines +248 to +253
Copy link
Copy Markdown
Member Author

@nathancoleman nathancoleman Nov 3, 2022

Choose a reason for hiding this comment

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

This was previously narrowed to the specific PodSecurityPolicy created for the API Gateway controller; however, the controller needs to be able to create Roles/RoleBindings that use whatever PSP is named in the GatewayClassConfig. Since you can't grant permissions that you don't have, the controller needs to be able to use more than just the PSP that applies to itself.

- apiGroups:
- rbac.authorization.k8s.io
resources:
- roles
- rolebindings
verbs:
- use
- create
- get
- list
- watch
Comment on lines +254 to +263
Copy link
Copy Markdown
Member Author

@nathancoleman nathancoleman Nov 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not a huge fan of allowing the API gateway controller to create Roles + RoleBindings; however, I don't see any other way to accomplish attaching a PodSecurityPolicy to each and every Deployment that the controller creates. Due to this, I've narrowed down the permissions as much as possible - note that it can't update existing resources - and only grant these permissions to the API gateway controller when global.enablePodSecurityPolicies.

Copy link
Copy Markdown
Contributor

@thisisnotashwin thisisnotashwin Nov 4, 2022

Choose a reason for hiding this comment

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

Thanks so much for the detail in the comment. I really appreciate it.

{{- end }}
{{- end }}
3 changes: 3 additions & 0 deletions charts/consul/templates/api-gateway-gatewayclassconfig.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ spec:
{{- if .Values.global.acls.manageSystemACLs }}
managed: true
method: {{ template "consul.fullname" . }}-k8s-auth-method
{{- if .Values.global.enablePodSecurityPolicies }}
podSecurityPolicy: {{ template "consul.fullname" . }}-api-gateway
Copy link
Copy Markdown
Member Author

@nathancoleman nathancoleman Nov 3, 2022

Choose a reason for hiding this comment

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

This specifies the name of the podSecurityPolicy that should apply to any Gateway using this GatewayClassConfig. The API gateway controller will then create a Role and RoleBinding to bind this PSP to the ServiceAccount that it creates.

This is intentionally pretty flexible so that a user doesn't need a code release from us if they, for example, wanted to create their own PodSecurityPolicy and have the API gateway controller use that. By allowing this, we let users tailor to their needs without requiring a code release from us.

{{- end }}
{{- end }}
{{- if .Values.global.tls.enabled }}
scheme: https
Expand Down
45 changes: 45 additions & 0 deletions charts/consul/templates/api-gateway-podsecuritypolicy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{{- if and .Values.apiGateway.enabled .Values.global.enablePodSecurityPolicies }}
Copy link
Copy Markdown
Member Author

@nathancoleman nathancoleman Nov 3, 2022

Choose a reason for hiding this comment

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

This is the PSP that will be applied to all Gateway deployments by default when global.enablePodSecurityPolicies=true. As I mentioned above, customers needing something more or less restrictive could create their own PodSecurityPolicy or modify this one and have the API gateway controller use it instead.

This is based on the PSP that we use for the API gateway controller with some slight modifications. Please review this for sensibility.

apiVersion: policy/v1beta1
kind: PodSecurityPolicy
metadata:
name: {{ template "consul.fullname" . }}-api-gateway
namespace: {{ .Release.Namespace }}
labels:
app: {{ template "consul.name" . }}
chart: {{ template "consul.chart" . }}
heritage: {{ .Release.Service }}
release: {{ .Release.Name }}
component: api-gateway-controller
spec:
privileged: false
# Required to prevent escalations to root.
allowPrivilegeEscalation: false
# This is redundant with non-root + disallow privilege escalation,
# but we can provide it for defense in depth.
requiredDropCapabilities:
- ALL
# Allow core volume types.
volumes:
- 'configMap'
- 'emptyDir'
- 'projected'
- 'secret'
- 'downwardAPI'
allowedCapabilities:
- NET_BIND_SERVICE
hostNetwork: false
hostIPC: false
hostPID: false
hostPorts:
- max: 65535
min: 1025
runAsUser:
rule: 'RunAsAny'
seLinux:
rule: 'RunAsAny'
supplementalGroups:
rule: 'RunAsAny'
fsGroup:
rule: 'RunAsAny'
readOnlyRootFilesystem: true
{{- end }}
16 changes: 14 additions & 2 deletions charts/consul/test/unit/api-gateway-controller-clusterrole.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,26 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "apiGateway/ClusterRole: uses PodSecurityPolicy with apiGateway.enabled=true and global.enablePodSecurityPolicies=true" {
@test "apiGateway/ClusterRole: can use podsecuritypolicies with apiGateway.enabled=true and global.enablePodSecurityPolicies=true" {
cd `chart_dir`
local actual=$(helm template \
-s templates/api-gateway-controller-clusterrole.yaml \
--set 'global.enablePodSecurityPolicies=true' \
--set 'apiGateway.enabled=true' \
--set 'apiGateway.image=foo' \
. | tee /dev/stderr |
yq '.rules[] | select((.resourceNames[0] == "release-name-consul-api-gateway-controller") and (.resources[0] == "podsecuritypolicies")) | length > 0' | tee /dev/stderr)
yq '.rules[] | select((.resources[0] == "podsecuritypolicies") and (.verbs[0] == "use")) | length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "apiGateway/ClusterRole: can create roles and rolebindings with apiGateway.enabled=true and global.enablePodSecurityPolicies=true" {
cd `chart_dir`
local actual=$(helm template \
-s templates/api-gateway-controller-clusterrole.yaml \
--set 'global.enablePodSecurityPolicies=true' \
--set 'apiGateway.enabled=true' \
--set 'apiGateway.image=foo' \
. | tee /dev/stderr |
yq '.rules[] | select((.resources[0] == "roles") and (.resources[1] == "rolebindings") and (.verbs | contains(["create","get","list","watch"]))) | length > 0' | tee /dev/stderr)
[ "${actual}" = "true" ]
}