excalq commented 4 months ago

Feature Request/Documentation Request

Note: This is somewhere between a bug report, feature request, and documentation suggestion. Feel free to re-categorize it as appropriate.

Is your feature request related to a problem? Please describe:

I'm using KCL to generate Crossplane manifests, and encountered a problem when using it to override labels. This is the handling of keys with special characters such as dots, slashes, and hyphens.

Here's a code sample for reproduction:

import yaml
import manifests

_crossplane_manifest_yaml = """
apiVersion: iam.aws.upbound.io/v1beta1
kind: Policy
metadata:
  name: crossplane-policy-OVERRIDE
  labels:
    platform.example.com/policy: crossplane-policy-OVERRIDE
    another-label: another-value
spec:
  providerConfigRef:
    name: OVERRIDE
  deletionPolicy: Orphan
  forProvider:
    description: Generated with Crossplane IaC
    policy: { INSERT JSON POLICY HERE }
"""

_crd_read = yaml.decode(_crossplane_manifest_yaml)

# Works, but is awkward/verbose
_crd_read.metadata.labels = _crd_read.metadata.labels | {'platform.example.com/policy' = "crossplane-policy-NEW-VALUE"}
# Also works, not documented (copilot suggested this)
_crd_read.metadata.labels.update({'platform.example.com/policy' = "crossplane-policy-NEW-VALUE"})

# Does not work (`^ expected one of ["identifier"] got platform.example.com/policy`)
_crd_read.metadata.labels.'platform.example.com/policy' = "crossplane-policy-NEW-VALUE"
# Same error as above
_crd_read.metadata.labels."platform.example.com/policy" = "crossplane-policy-NEW-VALUE"
# Does not work (`^ expected one of ["identifier"] got newline; ^ missing target in the assign statement`)
_crd_read.metadata.labels["platform.example.com/policy"] = "crossplane-policy-NEW-VALUE"

manifests.yaml_stream([_crd_read])

KCL Playground link

Describe the feature you'd like:

For good readability and maintainability, an intuitive way to override dict values is desired. Ideally something like _crd_read.metadata.labels."platform.example.com/policy" = "value".

Other means such as update() could be better documented. The union operator also works, but is hard to find in the docs, and in my opinion is less readable.

Describe alternatives you've considered:

Alternatives are shown above, with commends about which work, and which fail.

Interestingly, the closed issue https://github.com/kcl-lang/kcl/issues/1021 notes that this was "closed as completed", though no PR is linked. This functionality does not work in KCL 0.8.9-darwin-arm64

Teachability, Documentation, Adoption, Migration Strategy:

This would be very applicable to managing K8s manifests where YAML is the starting point. Of course starting from a KCL object avoids this situation, but there are many use cases where template YAML is necessary as a configuration language capability.

If update() is the best strategy, this should be prominent in the documentation. I was unable to find any occurrence of it, fortunately GitHub Copilot suggested it in an autocompletion.

Peefy commented 4 months ago

Very good suggestion. Thank you! @excalq

I prefer the statement _crd_read.metadata.labels["platform.example.com/policy"] = "crossplane-policy-NEW-VALUE" as a new feature to update config.

Besides, update() function is invalid in KCL, GitHub Copilot may have suggested a form of Python, but | is also the union operator for Python, which KCL has extended. 🤔

automation#2-use-kcl-cli-for-automation, but it is not yet supported in the KCL code. To maintain consistency, it should also be implemented in the KCL syntax for intuitive configuration updates.

Currently, we can write this code.

import yaml
import manifests

_crossplane_manifest_yaml = """
apiVersion: iam.aws.upbound.io/v1beta1
kind: Policy
metadata:
  name: crossplane-policy-OVERRIDE
  labels:
    platform.example.com/policy: crossplane-policy-OVERRIDE
    another-label: another-value
spec:
  providerConfigRef:
    name: OVERRIDE
  deletionPolicy: Orphan
  forProvider:
    description: Generated with Crossplane IaC
    policy: { INSERT JSON POLICY HERE }
"""

_crd_read = yaml.decode(_crossplane_manifest_yaml)

manifests.yaml_stream([_crd_read | {
    metadata.labels: {
        "platform.example.com/policy" = "crossplane-policy-NEW-VALUE"
    }
}])

excalq commented 4 months ago

I was mistaken about update()! There is no compiler error but it doesn't actually do anything. Thanks for the correction. The union works fine, though I find the brackets [] indexing more readable and concise. That's great that it is planned to be supported soon.

ghostsquad commented 2 months ago

Ya, running into the same issue, and I really expected to be able to do something like the 3 non-working examples above. Another use case is this:

_name = "foo"
a.b[_name].c = "bar"

kcl-lang / kcl

K8s YAML templating: Updating keys with dots/special characters #1396

Feature Request/Documentation Request