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.

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

Jump to bottom

Improve default security settings #439

Merged
merged 3 commits into from
Aug 11, 2023
Merged

Improve default security settings #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f13530
Improve default security settings
jawnsy Jul 28, 2023
3a1fb43
Mount scratch volume for /state path as well, to avoid errors
lfittl Aug 11, 2023
40ae58b
Helm Chart: Update README using helm-docs based on recent changes
lfittl Aug 11, 2023
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
14 changes: 10 additions & 4 deletions contrib/helm/pganalyze-collector/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ pganalyze statistics collector
| nameOverride | string | `""` | |
| nodeSelector | object | `{}` | |
| podAnnotations | object | `{}` | |
| podSecurityContext | object | `{}` | |
| podSecurityContext.runAsGroup | int | `1000` | |
| podSecurityContext.runAsNonRoot | bool | `true` | |
| podSecurityContext.runAsUser | int | `1000` | |
| podSecurityContext.seccompProfile.type | string | `"RuntimeDefault"` | |
| replicaCount | int | `1` | |
| resources.limits.cpu | string | `"1000m"` | |
| resources.limits.memory | string | `"1024Mi"` | |
Expand All @@ -36,14 +39,17 @@ pganalyze statistics collector
| secret.values | object | `{}` | Values to initialize the Secret with. Only applicable if create is true |
| securityContext.allowPrivilegeEscalation | bool | `false` | |
| securityContext.capabilities.drop[0] | string | `"ALL"` | |
| securityContext.readOnlyRootFilesystem | bool | `true` | |
| securityContext.runAsGroup | int | `1000` | |
| securityContext.runAsNonRoot | bool | `true` | |
| securityContext.runAsUser | int | `1000` | |
| securityContext.seccompProfile.type | string | `"RuntimeDefault"` | |
| serviceAccount.annotations | object | `{}` | Annotations to add to the service account |
| serviceAccount.create | bool | `true` | Specifies whether a service account should be created |
| serviceAccount.name | string | `""` | The name of the service account to use. If not set and create is true, a name is generated using the fullname template |
| tolerations | list | `[]` | |
| volumeMounts | list | `[]` | List of volume mounts to attach to the container |
| volumes | list | `[]` | List of volumes to attach to the pod |
| volumeMounts | list | `[{"mountPath":"/tmp","name":"scratch","subPath":"tmp"},{"mountPath":"/state","name":"scratch","subPath":"state"}]` | List of volume mounts to attach to the container |
| volumes | list | `[{"emptyDir":{},"name":"scratch"}]` | List of volumes to attach to the pod |

----------------------------------------------
Autogenerated from chart metadata using [helm-docs v1.10.0](https://github.com/norwoodj/helm-docs/releases/v1.10.0)
Autogenerated from chart metadata using [helm-docs v1.11.0](https://github.com/norwoodj/helm-docs/releases/v1.11.0)
1 change: 1 addition & 0 deletions contrib/helm/pganalyze-collector/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ spec:
labels:
{{- include "pganalyze-collector.selectorLabels" . | nindent 8 }}
spec:
enableServiceLinks: false
lfittl marked this conversation as resolved.
Show resolved Hide resolved
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
{{- toYaml . | nindent 8 }}
Expand Down
27 changes: 22 additions & 5 deletions contrib/helm/pganalyze-collector/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,24 @@ serviceAccount:

podAnnotations: {}

podSecurityContext: {}
podSecurityContext:
runAsNonRoot: true
runAsUser: 1000
runAsGroup: 1000
seccompProfile:
type: RuntimeDefault

securityContext:
capabilities:
drop:
- ALL
- ALL
readOnlyRootFilesystem: true
allowPrivilegeEscalation: false
runAsNonRoot: true
runAsUser: 1000
allowPrivilegeEscalation: false
runAsGroup: 1000
seccompProfile:
type: RuntimeDefault

resources:
limits:
Expand All @@ -74,10 +83,18 @@ resources:
memory: 1024Mi

# -- List of volumes to attach to the pod
volumes: []
volumes:
- name: scratch
emptyDir: {}

# -- List of volume mounts to attach to the container
volumeMounts: []
volumeMounts:
- mountPath: /tmp
name: scratch
subPath: tmp
Copy link
Member

@lfittl lfittl Jul 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Given the read-only root filesystem setting, I wonder if we also need to create a volume mount for /state?

(which is where the state file is written to - whilst that is mainly intended to support restarts outside of a container-based environment, it avoids errors that would likely occur when /state is no writeable)

See https://github.com/pganalyze/collector/blob/main/contrib/docker-entrypoint.sh#L23

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes, absolutely! I didn't know about this file

- mountPath: /state
name: scratch
subPath: state

nodeSelector: {}

Expand Down
Loading