CP 19442: CloudAccountId must be a string

[JonParsons11350]

By submitting a PR to this repository, you agree to the terms within the CloudZero Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

Problem

The issue is with cloudAccountId being a large integer. What happens is, before the cloudAccountId reaches our helm chart, it has changed into scientific notation.

Among others, you can see people discussing the issue here: helm/helm#1707

If cloudAccountId is being passed as an explicit argument, eg. --set cloudAccountId=123123123123123123123123123123
it works as expected

But when using -f to include a file that has the cloudAccountId, it gets converted to scientific notation.

Alternatives considered

Tried a few approaches that were non ideal.

1. In helm chart, cast the value to an integer (which works for scientific notation), and then use that value if it's a number > 0, otherwise assume it was a string. See the first commit on this branch for that attempt.
   This does not work if either a) the conversion to scientific notation lost precision, eg. 123123123123123123 ends up as like 123123123123123000 . or b) leading zero's are significant, which apparently they are in AWS.

2. Helm hooks - can't rely on these since customers are likely using tools like argo or spinaker rather than helm

Solution

We must mandate that cloudAccountId is a string. This prevents the issue, and gives the user immediate feedback about the issue.
It might be annoying for customers, but it also feels justifiable. Consider this description from CoPilot (emphasis mine):

AWS Account ID: This is typically a 12-digit integer, but when used in configurations, SDKs, or when specified in policies, it's treated as a string because leading zeros are significant.

GCP Project Number: This is an integer. However, like AWS Account IDs, when used programmatically or in configurations, it's often treated as a string to maintain consistency across various systems that might require string input.

Azure Subscription ID: This is a GUID (Globally Unique Identifier), which is not an integer. GUIDs are alphanumeric strings (including hyphens) that follow a specific structure, such as 12345678-1234-1234-1234-123456789abc. Therefore, Azure Subscription IDs are always treated as strings.

Testing

Running command like this:
helm template cz-prom-agent . --set apiKey=$api_key --set clusterName=jonp --set host=dev-api.cloudzero.com --set createSecret=true --set prometheus-node-exporter.enabled=true --set kube-state-metrics.enabled=true --set clusterName='jonp-test-cluster' -f extra-values.yaml | grep remote_write -A 2
with cloudAccountId set to integer gives this immediate feedback:

cloudzero-agent:
- cloudAccountId: Invalid type. Expected: string, given: integer

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

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

[dmepham]

@JonParsons11350

Mandate that cloudAccountId is a string - this prevents the issue, and gives the user immediate feedback about the issue.

I think this is a good solution given the drawbacks of trying to handle it within the chart

[roberthocking]

Looks Good

[josephbarnett]

Nice, Great write up!

[dmepham]

just a changelog request, looks great!

[dmepham]

I do think we're on 0.0.13 technically, we may just be missing the notes from .12. I think the eventual goal plan is to have github actions update this, but for now, would you mind bumping this to 0.13 and adding the release notes for .12 here?

[JonParsons11350]

Good catch!

[dmepham]

oh nice, I didn't know about that function