pulumi-kubernetes-operator
pulumi-kubernetes-operator copied to clipboard
pulumi
Support structured config
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
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] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Please view the PR build - https://github.com/pulumi/pulumi-kubernetes-operator/actions/runs/6802683612
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 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?
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.
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.
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
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
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
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
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
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
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