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

CP-20643 clean whitespace on cloudAccountId before use #66

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

josephbarnett
Copy link
Contributor

@josephbarnett josephbarnett commented Aug 2, 2024

Description

  1. Add cleanString helper to sanitize strings used as parameters
  2. Sanitize the cloudAccountId, host, clusterName, and region before use

Testing

  • This change adds test coverage for new/changed/fixed functionality

Manual Testing

  1. Deploy cluster using rancher

  2. Apply chart using the following values.yaml overrides:

cloudAccountId: |-
  1234567890
clusterName: local-workshop-cirrus-rancher
region: us-east-2
existingSecretName: cz-api-token
kube-state-metrics:
  enabled: false
prometheus-node-exporter:
  enabled: false

validator:
  serviceEndpoints:
    kubeStateMetrics: kube-state-metrics.monitoring.svc.cluster.local:8080
    prometheusNodeExporter: node-exporter-prometheus-node-exporter.monitoring.svc.cluster.local:9100
> Notice the intentional newlines added to cloudAccountId parameter above.
  1. Apply:
helm install cloudzero-agent . -f  rancher-agent-values.yaml --namespace $NS
  1. Validate the rendered value in the configMap (scrape config):
kubectl get configmap cloudzero-agent-configuration -n $NS -o yaml

Output:

apiVersion: v1
data:
  prometheus.yml: |-
    global:
      scrape_interval: 60s
    scrape_configs:
      - job_name: cloudzero-service-endpoints # kube_*, node_* metrics
        honor_labels: true
        honor_timestamps: true
        track_timestamps_staleness: false
        scrape_interval: 1m
        scrape_timeout: 10s
        scrape_protocols:
        - OpenMetricsText1.0.0
        - OpenMetricsText0.0.1
        - PrometheusText0.0.4
        metrics_path: /metrics
        scheme: http
        enable_compression: true
        follow_redirects: true
        enable_http2: true
        relabel_configs:
        - source_labels: [__meta_kubernetes_service_annotation_prometheus_io_scrape]
          separator: ;
          regex: "true"
          replacement: $1
          action: keep
        - source_labels: [__meta_kubernetes_service_annotation_prometheus_io_scrape_slow]
          separator: ;
          regex: "true"
          replacement: $1
          action: drop
        - source_labels: [__meta_kubernetes_service_annotation_prometheus_io_scheme]
          separator: ;
          regex: (https?)
          target_label: __scheme__
          replacement: $1
          action: replace
        - source_labels: [__meta_kubernetes_service_annotation_prometheus_io_path]
          separator: ;
          regex: (.+)
          target_label: __metrics_path__
          replacement: $1
          action: replace
        - source_labels: [__address__, __meta_kubernetes_service_annotation_prometheus_io_port]
          separator: ;
          regex: (.+?)(?::\d+)?;(\d+)
          target_label: __address__
          replacement: $1:$2
          action: replace
        - separator: ;
          regex: __meta_kubernetes_service_annotation_prometheus_io_param_(.+)
          replacement: __param_$1
          action: labelmap
        - separator: ;
          regex: __meta_kubernetes_service_label_(.+)
          replacement: $1
          action: labelmap
        - source_labels: [__meta_kubernetes_namespace]
          separator: ;
          regex: (.*)
          target_label: namespace
          replacement: $1
          action: replace
        - source_labels: [__meta_kubernetes_service_name]
          separator: ;
          regex: (.*)
          target_label: service
          replacement: $1
          action: replace
        - source_labels: [__meta_kubernetes_pod_node_name]
          separator: ;
          regex: (.*)
          target_label: node
          replacement: $1
          action: replace
        metric_relabel_configs:
        - source_labels: [__name__]
          regex: "^(kube_node_info|kube_node_status_capacity|kube_pod_container_resource_limits|kube_pod_container_resource_requests|kube_pod_labels|kube_pod_info|node_dmi_info)$"
          action: keep
        - action: labelkeep
          regex: "^(board_asset_tag|container|created_by_kind|created_by_name|image|instance|name|namespace|node|node_kubernetes_io_instance_type|pod|product_name|provider_id|resource|unit|uid|_.*|label_.*|app.kubernetes.io/*|k8s.*)$"
        kubernetes_sd_configs:
        - role: endpoints
          kubeconfig_file: ""
          follow_redirects: true
          enable_http2: true
      - job_name: cloudzero-nodes-cadvisor # container_* metrics
        honor_timestamps: true
        track_timestamps_staleness: false
        scrape_interval: 1m
        scrape_timeout: 10s
        scrape_protocols:
        - OpenMetricsText1.0.0
        - OpenMetricsText0.0.1
        - PrometheusText0.0.4
        metrics_path: /metrics
        scheme: https
        enable_compression: true
        authorization:
          type: Bearer
          credentials_file: /var/run/secrets/kubernetes.io/serviceaccount/token
        tls_config:
          ca_file: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
          insecure_skip_verify: true
        follow_redirects: true
        enable_http2: true
        relabel_configs:
        - separator: ;
          regex: __meta_kubernetes_node_label_(.+)
          replacement: $1
          action: labelmap
        - separator: ;
          regex: (.*)
          target_label: __address__
          replacement: kubernetes.default.svc:443
          action: replace
        - source_labels: [__meta_kubernetes_node_name]
          separator: ;
          regex: (.+)
          target_label: __metrics_path__
          replacement: /api/v1/nodes/$1/proxy/metrics/cadvisor
          action: replace
        - source_labels: [__meta_kubernetes_node_name]
          target_label: node
          action: replace
        metric_relabel_configs:
        - action: labelkeep
          regex: "^(board_asset_tag|container|created_by_kind|created_by_name|image|instance|name|namespace|node|node_kubernetes_io_instance_type|pod|product_name|provider_id|resource|unit|uid|_.*|label_.*|app.kubernetes.io/*|k8s.*)$"
        - source_labels: [__name__]
          regex: "^(container_cpu_usage_seconds_total|container_memory_working_set_bytes|container_network_receive_bytes_total|container_network_transmit_bytes_total)$"
          action: keep
        kubernetes_sd_configs:
        - role: node
          kubeconfig_file: ""
          follow_redirects: true
          enable_http2: true
    remote_write:
      - url: 'https://api.cloudzero.com/v1/container-metrics?cluster_name=local-workshop-cirrus-rancher&cloud_account_id=1234567890&region=us-east-2'
        authorization:
          credentials_file: /etc/config/prometheus/secrets/value
        write_relabel_configs:
          - source_labels: [__name__]
            regex: "^(kube_node_info|kube_node_status_capacity|kube_pod_container_resource_limits|kube_pod_container_resource_requests|kube_pod_labels|kube_pod_info|node_dmi_info|container_cpu_usage_seconds_total|container_memory_working_set_bytes|container_network_receive_bytes_total|container_network_transmit_bytes_total)$"
            action: keep
        metadata_config:
          send: false
kind: ConfigMap
metadata:
  annotations:
    meta.helm.sh/release-name: cloudzero-agent
    meta.helm.sh/release-namespace: cloudzero
  creationTimestamp: "2024-08-02T19:18:42Z"
  labels:
    app.kubernetes.io/component: server
    app.kubernetes.io/instance: cloudzero-agent
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: cloudzero-agent
    app.kubernetes.io/part-of: cloudzero-agent
    app.kubernetes.io/version: v2.50.1
    helm.sh/chart: cloudzero-agent-0.0.0-dev
  name: cloudzero-agent-configuration
  namespace: cloudzero
  resourceVersion: "451455"
  uid: 1289d4be-99b1-4638-9d1a-77905537cd4d

Checklist

  • I have added documentation for new/changed functionality in this PR
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not main

@josephbarnett josephbarnett requested a review from a team as a code owner August 2, 2024 19:36
Copy link
Collaborator

@dmepham dmepham left a comment

Choose a reason for hiding this comment

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

one small thing, but lgtm. is there a a reason this value in particular has this issue?

@josephbarnett
Copy link
Contributor Author

one small thing, but lgtm. is there a a reason this value in particular has this issue?

@dmepham

This is one of the strings we should be cleaning before use since we don't control if the user provides junk hidden characters before or after the value.

Since this is used to build the remote URL; any hidden characters may cause a remote write to fail every time...

Thus as a best practice we should be cleaning the input parameter strings... as well as validating them if we can.

This particular issue is born from a kustomize variable replacement issue, in comment with our schema enforcement that the account id is a string.

ie. it must have quotes if it looks like a number....

The specific user trying this using kustomize had to resort to the workaround, which introduces newlines around the provided ID - causing all HTTP calls to the remote write endpoint to fail (403).

This solves that.

@josephbarnett josephbarnett requested a review from dmepham August 2, 2024 23:32
Copy link
Contributor

@bdrennz bdrennz left a comment

Choose a reason for hiding this comment

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

Looks good!

@josephbarnett josephbarnett merged commit 734b6ef into develop Aug 5, 2024
2 checks passed
@josephbarnett josephbarnett deleted the CP-20643 branch August 5, 2024 13:42
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.

3 participants