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

aws-for-fluent-bit add ability to add securityContext #923

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

razorsk8jz
Copy link
Contributor

Issue

Currently aws-for-fluent-bit does not have the ability to add a securityContext which could cause companies with strict Pod Security Standards to not be able to use aws-for-fluent-bit

Description of changes

Added via Daemonset and created empty values for users to create their own policies

Checklist

  • Added/modified documentation as required (such as the README.md for modified charts)
  • Incremented the chart version in Chart.yaml for the modified chart(s)
  • Manually tested. Describe what testing was done in the testing section below
  • Make sure the title of the PR is a good description that can go into the release notes

Testing

Used helm template to ensure security policies showed up correctly - with and without a policy

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@joebowbeer joebowbeer left a comment

Choose a reason for hiding this comment

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

Requested change: drop [ALL]

Can the commented-out sample contexts be extended further? For example, why not add seccompProfile RuntimeDefault?

containerSecurityContext: {}
#capabilities:
# Drop:
# - all

This comment was marked as outdated.

@@ -32,6 +32,8 @@ helm delete aws-for-fluent-bit --namespace kube-system
| `image.repository` | Image to deploy | `amazon/aws-for-fluent-bit` | ✔
| `image.tag` | Image tag to deploy | `2.21.5`
| `image.pullPolicy` | Pull policy for the image | `IfNotPresent` | ✔
| `podSecurityContext` | Security Context for pod | `{}` |
| `containerSecurityContext` | Security Context for container | `{}` |
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: security context for Fluent Bit?

Copy link
Contributor Author

@razorsk8jz razorsk8jz Mar 20, 2023

Choose a reason for hiding this comment

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

I left these blank for users to decide their own policies - Also I am not sure what securityContext Fluent Bit needs to run correctly. Once this is change is in a think a separate PR could be used to determine what a default standard policy should look like.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant the text. The description is "Security Context for container" and we could be more descriptive

Copy link
Contributor

@joebowbeer joebowbeer left a comment

Choose a reason for hiding this comment

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

s/All/ALL/

# allowPrivilegeEscalation: false
# capabilities:
# Drop:
# - All
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# - All
# - ALL

https://kubernetes.io/docs/concepts/security/pod-security-standards/

Allowed Values

Any list of capabilities that includes ALL

@razorsk8jz
Copy link
Contributor Author

Fixed issues and squashed all commits into one

Copy link
Contributor

@joebowbeer joebowbeer left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@PettitWesley PettitWesley left a comment

Choose a reason for hiding this comment

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

147dda285fd4:aws-for-fluent-bit wppttt$ helm template --dry-run .
---
# Source: aws-for-fluent-bit/templates/serviceaccount.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
  name: release-name-aws-for-fluent-bit
  namespace: default
  labels:
    helm.sh/chart: aws-for-fluent-bit-0.1.24
    app.kubernetes.io/name: aws-for-fluent-bit
    app.kubernetes.io/instance: release-name-aws-for-fluent-bit
    app.kubernetes.io/version: "2.28.4"
    app.kubernetes.io/managed-by: Helm
---
# Source: aws-for-fluent-bit/templates/configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: release-name-aws-for-fluent-bit
  namespace: default
  labels:
    helm.sh/chart: aws-for-fluent-bit-0.1.24
    app.kubernetes.io/name: aws-for-fluent-bit
    app.kubernetes.io/instance: release-name-aws-for-fluent-bit
    app.kubernetes.io/version: "2.28.4"
    app.kubernetes.io/managed-by: Helm
data:
  fluent-bit.conf: |
    [SERVICE]
        Parsers_File /fluent-bit/parsers/parsers.conf
    [INPUT]
        Name              tail
        Tag               kube.*
        Path              /var/log/containers/*.log
        DB                /var/log/flb_kube.db
        Parser            docker
        Docker_Mode       On
        Mem_Buf_Limit     5MB
        Skip_Long_Lines   On
        Refresh_Interval  10
    [FILTER]
        Name                kubernetes
        Match               kube.*
        Kube_URL            https://kubernetes.default.svc.cluster.local:443
        Merge_Log           On
        Merge_Log_Key       data
        Keep_Log            On
        K8S-Logging.Parser  On
        K8S-Logging.Exclude On
        Buffer_Size         32k
    [OUTPUT]
        Name                  cloudwatch
        Match                 *
        region                us-east-1
        log_group_name        /aws/eks/fluentbit-cloudwatch/logs
        log_stream_prefix     fluentbit-
        auto_create_group     true
    [OUTPUT]
        Name            firehose
        Match           *
        region          us-east-1
        delivery_stream my-stream
    [OUTPUT]
        Name            kinesis
        Match           *
        region          us-east-1
        stream          my-kinesis-stream-name
        partition_key   container_id
    [OUTPUT]
        Name            es
        Match           *
        AWS_Region      us-east-1
        AWS_Auth        On
        Port            443
        TLS             On
        Retry_Limit     6
        Replace_Dots    On
---
# Source: aws-for-fluent-bit/templates/clusterrole.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: release-name-aws-for-fluent-bit
rules:
  - apiGroups: [""]
    resources:
      - namespaces
      - pods
      - pods/logs
      - nodes
      - nodes/proxy
    verbs: ["get", "list", "watch"]
  - apiGroups: ["policy"]
    resources: ["podsecuritypolicies"]
    verbs: ["use"]
    resourceNames:
      - release-name-aws-for-fluent-bit
---
# Source: aws-for-fluent-bit/templates/clusterrolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: release-name-aws-for-fluent-bit
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: release-name-aws-for-fluent-bit
subjects:
  - kind: ServiceAccount
    name: release-name-aws-for-fluent-bit
    namespace: default
---
# Source: aws-for-fluent-bit/templates/daemonset.yaml
apiVersion: apps/v1
kind: DaemonSet
metadata:
  name: release-name-aws-for-fluent-bit
  namespace: default
  labels:
    helm.sh/chart: aws-for-fluent-bit-0.1.24
    app.kubernetes.io/name: aws-for-fluent-bit
    app.kubernetes.io/instance: release-name-aws-for-fluent-bit
    app.kubernetes.io/version: "2.28.4"
    app.kubernetes.io/managed-by: Helm
spec:
  updateStrategy:
    type: RollingUpdate
  selector:
    matchLabels:
      app.kubernetes.io/name: aws-for-fluent-bit
      app.kubernetes.io/instance: release-name-aws-for-fluent-bit
  template:
    metadata:
      annotations:
        checksum/config: 25ef410fa8732dd74058bbf9ef985734b1e2136d1ff1deb8e0e3b3b948154957
      labels:
        app.kubernetes.io/name: aws-for-fluent-bit
        app.kubernetes.io/instance: release-name-aws-for-fluent-bit
    spec:
      serviceAccountName: release-name-aws-for-fluent-bit
      securityContext:
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
        seccompProfile:
          type: RuntimeDefault
      dnsPolicy: ClusterFirst
      containers:
        - name: aws-for-fluent-bit
          imagePullPolicy: IfNotPresent
          image: "public.ecr.aws/aws-observability/aws-for-fluent-bit:2.21.5"
          securityContext:
            Drop:
            - ALL
            allowPrivilegeEscalation: false
            capabilities: null
          volumeMounts:
            - name: fluentbit-config
              mountPath: /fluent-bit/etc/
            - mountPath: /var/log
              name: varlog
            - mountPath: /var/lib/docker/containers
              name: varlibdockercontainers
              readOnly: true
          resources:
            limits:
              memory: 250Mi
            requests:
              cpu: 50m
              memory: 50Mi
      volumes:
        - name: fluentbit-config
          configMap:
            name: release-name-aws-for-fluent-bit
        - hostPath:
            path: /var/log
          name: varlog
        - hostPath:
            path: /var/lib/docker/containers
          name: varlibdockercontainers

@PettitWesley PettitWesley merged commit 785a71d into aws:master Mar 20, 2023
@PettitWesley
Copy link
Contributor

@razorsk8jz Thanks. I will perform a release soonish for this and for the recent C plugin addition.

@joebowbeer
Copy link
Contributor

@PettitWesley sorry for not catching this sooner, but drop needs to be lowercase

The output above is invalid, given that Drop is uppercase and appears at the wrong level. Should be:

          securityContext:
            allowPrivilegeEscalation: false
            capabilities:
              drop:
              - ALL

@PettitWesley
Copy link
Contributor

@joebowbeer I already started staging a release: https://github.com/aws/eks-charts/releases/edit/untagged-ce896c6d8004d3020e49

Let's submit a PR real quick to fix this. Can do it within a few hours unless you're already working on it.

@joebowbeer
Copy link
Contributor

@PettitWesley #924

@razorsk8jz
Copy link
Contributor Author

Thanks @joebowbeer and @PettitWesley, i was out when i saw this issue come in

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.

4 participants