Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

Allow projected volumes for elasticsearch, logstash and metricbeat #496

Conversation

krichter722
Copy link
Contributor

@krichter722 krichter722 commented Feb 24, 2020

Projected volumes are the default volume type on Google Kubernetes Engine (GKE) with cluster >= 1.16. This PR allows to use them out-of-the-box more easily if pod security policy is enabled. This is only relevant for clusters with pod security policy enabled. By default >= 1.16 clusters should work with this PR.

Furthermore, it adds a missing pod security policy for metricbeat for this case.

  • Chart version not bumped (the versions are all bumped and released at the same time)
  • README.md updated with any new values or changes
  • Updated template tests in ${CHART}/tests/*.py
  • Updated integration tests in ${CHART}/examples/*/test/goss.yaml

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@krichter722 krichter722 force-pushed the controller-psp-projected-volume branch 3 times, most recently from bab6cab to 510f33b Compare March 1, 2020 22:46
@jmlrt jmlrt added the enhancement New feature or request label Mar 6, 2020
@krichter722 krichter722 force-pushed the controller-psp-projected-volume branch 7 times, most recently from f1519d9 to 69f69a4 Compare March 14, 2020 13:12
@krichter722 krichter722 force-pushed the controller-psp-projected-volume branch 2 times, most recently from eed0f62 to 52c3449 Compare March 15, 2020 13:03
@krichter722 krichter722 changed the title Allow projected volumes in elasticsearch/values.yaml Allow projected volumes for elasticsearch, logstash and metricbeat Mar 15, 2020
@krichter722 krichter722 force-pushed the controller-psp-projected-volume branch from 52c3449 to d4402df Compare May 13, 2020 23:06
Allow specification of PodSecurityPolicy in metricbeat/values.yml

Signed-off-by: Karl-Philipp Richter <krichter@posteo.de>
@krichter722 krichter722 force-pushed the controller-psp-projected-volume branch from d4402df to c68b80e Compare May 21, 2020 21:13
@jmlrt
Copy link
Member

jmlrt commented May 28, 2020

Hi @krichter722, thank you for this PR.
Can you add more details about your use case for projected volumes with Elasticsearch, Logstach and Metricbeat?

@jmlrt
Copy link
Member

jmlrt commented May 29, 2020

Thanks for the details. Do you have any source mentioning that projected volumes are required on GKE 1.16? I didn't find any and would be greatly interested by that.

@krichter722
Copy link
Contributor Author

@jmlrt No, they seem to be the default if nothing else is specified. Maybe the problem that the deployment on a cluster with pod security policy enabled fails with default values can be solved differently.

@jmlrt
Copy link
Member

jmlrt commented Jun 2, 2020

FYI, we just merged #635 which is adding tests on K8S 1.16.
As we don't have any automated integration tests with pod policy, I'd like to take some time this week for some manual tests, then we should be good to merge this PR.

@jmlrt
Copy link
Member

jmlrt commented Jun 2, 2020

jenkins test this please

@krichter722
Copy link
Contributor Author

As we don't have any automated integration tests with pod policy, I'd like to take some time this week for some manual tests, then we should be good to merge this PR.

@jmlrt There's no rush. I you have a plan for 1.16 and maybe even 1.17 tests, you should prioritize them and save the manual tests.

@jmlrt
Copy link
Member

jmlrt commented Jun 16, 2020

I did some tests with GKE 1.16 and PSP and was able to deploy Elasticsearch and Logstash charts without projected volumes. Can you post your other values?

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.3", GitCommit:"2e7996e3e2712684bc73f0dec0200d64eec7fe40", GitTreeState:"clean", BuildDate:"2020-05-21T14:50:54Z", GoVersion:"go1.14.3", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"16+", GitVersion:"v1.16.9-gke.2", GitCommit:"4751ff766b3f6dbf6c6a63394a909e8108e89744", GitTreeState:"clean", BuildDate:"2020-05-08T16:44:50Z", GoVersion:"go1.13.9b4", Compiler:"gc", Platform:"linux/amd64"}

$ helm version
version.BuildInfo{Version:"v3.2.4", GitCommit:"0ad800ef43d3b826f31a5ad8dfbb4fe05d143688", GitTreeState:"clean", GoVersion:"go1.13.12"}

$ helm ls                 
NAME            NAMESPACE       REVISION        UPDATED                                 STATUS          CHART                   APP VERSION
elasticsearch   default         2               2020-06-16 15:01:01.938775 +0200 CEST   deployed        elasticsearch-7.7.1     7.7.1      
logstash        default         1               2020-06-16 15:19:10.490404 +0200 CEST   deployed        logstash-7.7.0          7.7.0 

$ helm get values elasticsearch
USER-SUPPLIED VALUES:
podSecurityPolicy:
  create: true
  spec:
    fsGroup:
      rule: RunAsAny
    privileged: true
    runAsUser:
      rule: RunAsAny
    seLinux:
      rule: RunAsAny
    supplementalGroups:
      rule: RunAsAny
    volumes:
    - secret
    - configMap
    - persistentVolumeClaim
rbac:
  create: true

$ helm get values logstash     
USER-SUPPLIED VALUES:
podSecurityPolicy:
  create: true
  spec:
    fsGroup:
      rule: RunAsAny
    privileged: true
    runAsUser:
      rule: RunAsAny
    seLinux:
      rule: RunAsAny
    supplementalGroups:
      rule: RunAsAny
    volumes:
    - secret
    - configMap
    - persistentVolumeClaim
rbac:
  create: true

$ kubectl get pods      
NAME                     READY   STATUS    RESTARTS   AGE
elasticsearch-master-0   1/1     Running   0          20m
elasticsearch-master-1   1/1     Running   0          20m
elasticsearch-master-2   1/1     Running   0          20m
logstash-logstash-0      1/1     Running   0          2m44s

$ kubectl describe podsecuritypolicies.policy elasticsearch-master
Name:         elasticsearch-master
Namespace:    
Labels:       app=elasticsearch-master
              app.kubernetes.io/managed-by=Helm
              chart=elasticsearch-7.7.1
              heritage=Helm
              release=elasticsearch
Annotations:  meta.helm.sh/release-name: elasticsearch
              meta.helm.sh/release-namespace: default
API Version:  policy/v1beta1
Kind:         PodSecurityPolicy
Metadata:
  Creation Timestamp:  2020-06-16T12:53:29Z
  Resource Version:    3339
  Self Link:           /apis/policy/v1beta1/podsecuritypolicies/elasticsearch-master
  UID:                 59ddb3e9-0e59-4ee3-9231-e04cdc7b4840
Spec:
  Allow Privilege Escalation:  true
  Fs Group:
    Rule:      RunAsAny
  Privileged:  true
  Run As User:
    Rule:  RunAsAny
  Se Linux:
    Rule:  RunAsAny
  Supplemental Groups:
    Rule:  RunAsAny
  Volumes:
    secret
    configMap
    persistentVolumeClaim
Events:  <none>

$ kubectl describe podsecuritypolicies.policy logstash-logstash   
Name:         logstash-logstash
Namespace:    
Labels:       app=logstash-logstash
              app.kubernetes.io/managed-by=Helm
              chart=logstash
              heritage=Helm
              release=logstash
Annotations:  meta.helm.sh/release-name: logstash
              meta.helm.sh/release-namespace: default
API Version:  policy/v1beta1
Kind:         PodSecurityPolicy
Metadata:
  Creation Timestamp:  2020-06-16T13:19:11Z
  Resource Version:    12509
  Self Link:           /apis/policy/v1beta1/podsecuritypolicies/logstash-logstash
  UID:                 873fb69d-6a73-495c-ac09-94f7ac080ebf
Spec:
  Allow Privilege Escalation:  true
  Fs Group:
    Rule:      RunAsAny
  Privileged:  true
  Run As User:
    Rule:  RunAsAny
  Se Linux:
    Rule:  RunAsAny
  Supplemental Groups:
    Rule:  RunAsAny
  Volumes:
    secret
    configMap
    persistentVolumeClaim
Events:  <none>

Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

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

Metricbeat ClusterRole needs a rule for podsecuritypolicies resource:

{{- if .Values.managedServiceAccount }}
{{- $fullName := include "metricbeat.fullname" . -}}
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
  name: {{ template "metricbeat.serviceAccount" . }}-cluster-role
  labels:
    app: "{{ template "metricbeat.fullname" . }}"
    chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
    heritage: {{ .Release.Service | quote }}
    release: {{ .Release.Name | quote }}
rules:
  - apiGroups:
      - extensions
    resources:
      - podsecuritypolicies
    resourceNames:
      {{- if eq .Values.podSecurityPolicy.name "" }}
      - {{ $fullName | quote }}
      {{- else }}
      - {{ .Values.podSecurityPolicy.name | quote }}
      {{- end }}
    verbs:
      - use
    {{ toYaml .Values.clusterRoleRules | nindent 2 -}}
{{- end -}}

rule: RunAsAny
supplementalGroups:
rule: RunAsAny
volumes:
Copy link
Member

Choose a reason for hiding this comment

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

hostPath is also required for Metricbeat daemonset

@jmlrt
Copy link
Member

jmlrt commented Jun 16, 2020

During my tests metricbeat-kube-state-metrics was also not matching pod security policy and pods not starting. I think we need to create a second PodSecurityPolicy specific for kube-state-metrics.

@@ -114,6 +114,7 @@ podSecurityPolicy:
- secret
- configMap
- persistentVolumeClaim
- projected
Copy link
Member

Choose a reason for hiding this comment

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

While projected doesn't seem required in my tests with GKE 1.16 it is mentionned as part of the recommended minimum set of allowed volumes for new PSPs in https://kubernetes.io/docs/concepts/policy/pod-security-policy/#volumes-and-file-systems.

Note that we could also add downwardAPI and emptyDir which are also part of the recommendations (same for the other charts)

Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

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

Also can you please add podSecurityPolicy value description to the README doc and some python tests for the pod security policy like in

def test_pod_security_policy():
## Make sure the default config is not creating any resources
config = ""
resources = ("role", "rolebinding", "serviceaccount", "podsecuritypolicy")
r = helm_template(config)
for resource in resources:
assert resource not in r
assert (
"serviceAccountName" not in r["statefulset"][name]["spec"]["template"]["spec"]
)
## Make sure all the resources are created with default values
config = """
rbac:
create: true
serviceAccountName: ""
podSecurityPolicy:
create: true
name: ""
"""
r = helm_template(config)
for resource in resources:
assert resource in r
assert r["role"][name]["rules"][0] == {
"apiGroups": ["extensions"],
"verbs": ["use"],
"resources": ["podsecuritypolicies"],
"resourceNames": [name],
}
assert r["rolebinding"][name]["subjects"] == [
{"kind": "ServiceAccount", "namespace": "default", "name": name}
]
assert r["rolebinding"][name]["roleRef"] == {
"apiGroup": "rbac.authorization.k8s.io",
"kind": "Role",
"name": name,
}
assert (
r["statefulset"][name]["spec"]["template"]["spec"]["serviceAccountName"] == name
)
psp_spec = r["podsecuritypolicy"][name]["spec"]
assert psp_spec["privileged"] is True
def test_external_pod_security_policy():
## Make sure we can use an externally defined pod security policy
config = """
rbac:
create: true
serviceAccountName: ""
podSecurityPolicy:
create: false
name: "customPodSecurityPolicy"
"""
resources = ("role", "rolebinding")
r = helm_template(config)
for resource in resources:
assert resource in r
assert r["role"][name]["rules"][0] == {
"apiGroups": ["extensions"],
"verbs": ["use"],
"resources": ["podsecuritypolicies"],
"resourceNames": ["customPodSecurityPolicy"],
}
def test_external_service_account():
## Make sure we can use an externally defined service account
config = """
rbac:
create: false
serviceAccountName: "customServiceAccountName"
podSecurityPolicy:
create: false
name: ""
"""
resources = ("role", "rolebinding", "serviceaccount")
r = helm_template(config)
assert (
r["statefulset"][name]["spec"]["template"]["spec"]["serviceAccountName"]
== "customServiceAccountName"
)
# When referencing an external service account we do not want any resources to be created.
for resource in resources:
assert resource not in r

@jmlrt jmlrt mentioned this pull request Jun 16, 2020
4 tasks
@botelastic
Copy link

botelastic bot commented Sep 14, 2020

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. To track this PR (even if closed), please open a corresponding issue if one does not already exist.

@botelastic
Copy link

botelastic bot commented Dec 14, 2020

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. To track this PR (even if closed), please open a corresponding issue if one does not already exist.

@jmlrt
Copy link
Member

jmlrt commented Dec 23, 2020

closing as stale

@jmlrt jmlrt closed this Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants