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

Support for Federated Storage via yaml #3411

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

williamkubecost
Copy link
Contributor

@williamkubecost williamkubecost commented May 13, 2024

What does this PR change?

Allows support for specifying federated storage via YAML values. Also removes previous merge conflict.

Does this PR rely on any other PRs?

I do not believe so.

How does this PR impact users? (This is the kind of thing that goes in release notes!)

Many customers rely on automation in the form of IaC and CI/CD pipelines to deploy their applications.
It can be a burden to manage and maintain a secret using that approach, including updating values, base64 encoding, deploying, etc.

Links to Issues or tickets this PR addresses or fixes

Closes #3216

What risks are associated with merging this PR? What is required to fully test this PR?

I don't believe there is a real risk, I added a new value for users to configure. I did not modify any pre-existing values.

How was this PR tested?

  1. Created a test values.yaml file similar to below:
federatedStorageConfig: |-
  configs:
    - type: S3
      config:
        bucket: kubecost-UNIQUE_NAME-metrics
        endpoint: s3.amazonaws.com
        region: us-east-2
        insecure: false
        signature_version2: false
        put_user_metadata:
          X-Amz-Acl: bucket-owner-full-control
        http_config:
          idle_conn_timeout: 90s
          response_header_timeout: 2m
          insecure_skip_verify: false
        trace:
          enable: true
        part_size: 134217728
    - type: GCS
      config:
        bucket: GCP_object_store_bucket
        service_account:
          type: service_account
          project_id: GCP_object_store_project_id
          private_key_id: GCP_object_store_private_key_id
          private_key: GCP_object_store_privateKEY
          client_email: GCP_object_store_client_email
          client_id: GCP_object_store_client_id
          auth_uri: https://accounts.google.com/o/oauth2/auth
          token_uri: https://oauth2.googleapis.com/token
          auth_provider_x509_cert_url: https://www.googleapis.com/oauth2/v1/certs
          client_x509_cert_url: GCP_object_store_client_x509_cert_url
    - type: AZURE
      config:
        storage_account: AZ_object_store_storage_account
        storage_account_key: AZ_object_store_storageAccount_key
        container: AZ_object_store_container
        max_retries: 0


  1. Used the following command to test the template and create a rendered output of all the yaml files.
helm template cost-analyzer --values test-values.yaml > rendered.yaml 
  1. Checked that the federated storage secret was part of the rendered.yaml
grep -A 15 "federated-store" rendered.yaml                                                               
# Source: cost-analyzer/templates/federated-store.yaml
apiVersion: v1
kind: Secret
type: Opaque
metadata:
  name: federated-store
  namespace: default
  labels:
    app.kubernetes.io/name: cost-analyzer
    helm.sh/chart: cost-analyzer-2.2.0
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/managed-by: Helm
    app: cost-analyzer
data:
  federated-store.yaml: 
---

  1. I also packed the helm chart locally, and deployed kubecost via kind and can see the secret is deployed there as well.
k get secrets -n kubecost | grep -i federated                                        
federated-store                          Opaque                                s
k describe secret federated-store -n kubecost                                       
Name:         federated-store
Namespace:    kubecost
Labels:       app=cost-analyzer
              app.kubernetes.io/instance=kubecost
              app.kubernetes.io/managed-by=Helm
              app.kubernetes.io/name=cost-analyzer
              helm.sh/chart=cost-analyzer-2.2.0
Annotations:  meta.helm.sh/release-name: kubecost
              meta.helm.sh/release-namespace: kubecost

Type:  Opaque

Data
====
federated-store.yaml:  1576 bytes

Have you made an update to documentation? If so, please provide the corresponding PR.

No, but I'm open to discussion about this if we feel it's needed!

Copy link

gitguardian bot commented May 13, 2024

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
7414 Triggered Google API Key db78bfa cost-analyzer/templates/cost-analyzer-deployment-template.yaml View secret
7414 Triggered Google API Key db78bfa cost-analyzer/templates/_helpers.tpl View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@williamkubecost
Copy link
Contributor Author

@chipzoller I ended up closing the original PR I created, mainly because I messed up the merge conflict. I know you mentioned having this under kubecostProductConfigs I left this under kubecostModel as that was where we were originally asking for the kubecostModel.federatedStorageConfigSecret

cost-analyzer/values.yaml Outdated Show resolved Hide resolved
Signed-off-by: Will <156370195+williamkubecost@users.noreply.github.com>
@williamkubecost williamkubecost changed the title Williamg/federated storage json Support for Federated Storage YAML May 23, 2024
Signed-off-by: Will <156370195+williamkubecost@users.noreply.github.com>
…storage-json' into williamg/federated-storage-json
Signed-off-by: Will <156370195+williamkubecost@users.noreply.github.com>
@thomasvn
Copy link
Member

@williamkubecost Nice progress! When you test with the following values.txt, what does the federated-store.yaml look like when you base64 decode it?

I believe that a federated-store.yaml can only contain a configuration for a single cloud provider. Additionally it must follow the format shown here https://github.com/kubecost/poc-common-configurations/blob/main/etl-federation-aggregator/federated-store.yaml.

@williamkubecost
Copy link
Contributor Author

@williamkubecost Nice progress! When you test with the following values.txt, what does the federated-store.yaml look like when you base64 decode it?

I believe that a federated-store.yaml can only contain a configuration for a single cloud provider. Additionally it must follow the format shown here https://github.com/kubecost/poc-common-configurations/blob/main/etl-federation-aggregator/federated-store.yaml.

I was able to test it with the values you provided, it seems like it worked without issue. In regards to have multiple cloud providers, I really only have them there as an example for each major provider 😄

@williamkubecost williamkubecost changed the title Support for Federated Storage YAML Support for Federated Storage via yaml Jun 1, 2024
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.

[Feature] Add support for populating federated-store.yaml via values.yaml
2 participants