Support structured config

[ljtfreitas]

Context

Currently structured configs are not supported properly by Pulumi k8s operator.

As Pulumi programs can be configured using yaml files, it looks fair to be able to configure a structured, complex config using the Stack CRD as well.

Proposed changes

This PR proposes two approaches to support structured config:

makes the config field support a JSON as value

It could be useful to keep compatibility with older versions. Declaring the config field as a map[string]apiextensionsv1.JSON, existing support to inline fields keeps working (as aws:region: whatever) and expanding the config to a more complex object also works.

To make this work with Pulumi Automation API, in case the value is a json, the same rules used by Pulumi CLI to handle structured configs are applied. The implementation flattens all object keys to generate Pulumi-CLI compatible key names. For example:

config:
  my-key: my-value
  my-list:
    - a
    - b
    - c
  aws:region: us-east-1
  aws:assumeRole:
     roleArn: my-role-arn
     sessionName: my-assumed-session-name

it will generate these config keys/values:

my-key: my-value
my-list[0]: a
my-list[1]: b
my-list[2]: c
aws:region: us-east-1
aws:assumeRole.roleArn: my-role-arn
aws:assumeRole.sessionName: my-assumed-session-name

and all those keys are send through Automation API as Path: true

adds a new field configRefs

adds to CRD a new field called configRefs, that works in a similar way that EnvRefs or SecretRefs, adding two new possible values: configmap and structured

configmap adds support to read a K8s ConfigMap and get a specific key to use as a config value structured is a JSON value that can be used to pass a structured configuration, following the same previous pattern from the config field.

integration tests

I was not able to make the tests pass in my local environment but right now it's just an initial implementation proposal. I can keep working on that if you people think is worth it.

Related issues (optional)

https://github.com/pulumi/pulumi-kubernetes-operator/issues/258

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[rquitales]

/run-acceptance-tests

[github-actions[bot]]

Please view the PR build - https://github.com/pulumi/pulumi-kubernetes-operator/actions/runs/6802683612

[Sergey-Kizimov]

Any updates? it would help us if this feature will be implemented.

[ljtfreitas]

Hi friends, any news about it?

[rquitales]

Hi friends, any news about it?

@ljtfreitas I apologize for the delay in reviewing this. Unfortunately, it fell off my radar and got lost in my GitHub notifications. I've provided some feedback, and overall, I believe this is a promising direction that we should push to completion soon to resolve a crucial user journey. My main concern lies with the type change and its potential downstream implications. As a user of this feature, what are your thoughts on keeping the type as a string? The structured config could still be represented as a string literal of the desired JSON configuration. While this may introduce some additional friction, I am inclined to maintain consistency with our CRD spec. WDYT?

[ljtfreitas]

Hi friends, any news about it?

@ljtfreitas I apologize for the delay in reviewing this. Unfortunately, it fell off my radar and got lost in my GitHub notifications. I've provided some feedback, and overall, I believe this is a promising direction that we should push to completion soon to resolve a crucial user journey. My main concern lies with the type change and its potential downstream implications. As a user of this feature, what are your thoughts on keeping the type as a string? The structured config could still be represented as a string literal of the desired JSON configuration. While this may introduce some additional friction, I am inclined to maintain consistency with our CRD spec. WDYT?

Hi @rquitales , don't worry.

About the question, yes, moving from string to map[string]apiextensionsv1.JSON is a breaking change, no doubt about it. I changed it as a potential idea but we need to think carefully about it. In other hand, a string field does not have a good ergonomic, I guess...we could put a json content, yes, but the program code would be required to handle it. Using the native config format from Pulumi just sounds more natural and ergonomic.

Maybe the approach using a new field, which I've suggested as configRefs, can be safer. config could stay untouched and more complex configurations could use the new field. An example:

apiVersion: pulumi.com/v1
kind: Stack
metadata:
  name: my-stack
spec:
    configRefs:
        aws:
            type: structured
            value:
              region: us-east-1
              assumeRole:
                roleArn: my-role-arn
                sessionName: my-assumed-session-name
        my-key:
          type: literal
          value: my-value
        my-list:
          type: structured
          value: [a, b, c]

The current implementation of this pull request would generate these config values:

my-key: my-value
my-list[0]: a
my-list[1]: b
my-list[2]: c
aws:region: us-east-1
aws:assumeRole.roleArn: my-role-arn
aws:assumeRole.sessionName: my-assumed-session-name

(actually I need to write a test to be sure about it 😅. but it's the idea).

Looking now the configRefs looks more complex than I've thought. The potential advantage here is be able to use env vars and secrets as config values (and I've introduced a new ConfigMap support).

WDYT? Maybe introducing a new field could be a better path?

[EronWright]

Regarding the API changes, I believe we need to ensure that we're following the rules of CRD schemas (ref). In particular, the list of configuration properties should be expressed as a list (with a merge key), not as a map.

[ljtfreitas]

Regarding the API changes, I believe we need to ensure that we're following the rules of CRD schemas (ref). In particular, the list of configuration properties should be expressed as a list (with a merge key), not as a map.

Sorry, not sure if I understood @EronWright . I don't get how passing config as a map would violate CRD rules.

Anyway, sounds better to create a new field for structured config in order to don't break the current API. I can keep working on that (my second suggestion is about it) and refining the implementation, if you folks agree.

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[ljtfreitas]

Hi @rquitales @EronWright , sorry for the delay. I rethink some things about the concerns and opened a new pull request: https://github.com/pulumi/pulumi-kubernetes-operator/pull/575