pulumi / pulumi-kubernetes-operator

A Kubernetes Operator that automates the deployment of Pulumi Stacks
Apache License 2.0
213 stars 54 forks source link

Support structured config #575

Open ljtfreitas opened 2 months ago

ljtfreitas commented 2 months ago

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

The proposed approach is to introduce a new field called configRefs, rather than change the current config one.

configRefs has a format close to other fields like envRefs and secretsRefs, supporting the same env and filesystem options as well (secret is not supported because just use secretsRefs looks better), and two new options:

ConfigLiteralRef

ConfigLiteralRef refers to both a simple or possibly structured, nested value in YAML format, in the same way that people would put in regular Pulumi.<stack>.yaml files:

configRefs:
   stack-config:
     type: Literal
     value:
        my-config:
          my-nested-field:
             field: "value"

The value content will be handled as a map of any type of value; so just simple key-pairs in YAML format are supported as well:

configRefs:
   stack-config:
     type: Literal
     value:
       field-a: value-a
       field-b: value-b

ConfigMapRef

ConfigMapRef refers to a Kubernetes ConfigMap. The implementation assumes that the content from the ConfigMap's key is a structured, nested content in YAML format. The idea is to enable people to just put their configs from Pulumi.<stack>.yaml files in a ConfigMap, and the operator will be able to handle it.

So given a ConfigMap called my-stack-config, like that:

apiVersion: v1
kind: ConfigMap
metadata:
  name: my-stack-config
data:
  Pulumi.my-stack.yaml: |
    my-config:
      my-nested-field:
          field: "value"

It could be consumed as a config to a Pulumi stack like that:

configRefs:
   stack-config:
     type: ConfigMap
     configmap:
       name: my-stack-config
       key: Pulumi.my-stack.yaml

Configs merge strategy

All configs resolved from configRefs fields will be merged with the ones from the config field. configRefs resolved key-values will override config ones with the same key.

Configs set strategy

All configs will be set using the path flag. So the config field will start to support nested values as well.

It's especially useful for some complex fields from providers. An example:

config:
   aws:assumeRole.roleArn: arn/my-aws-arn

Passing the value above with the --path flag, it will be configured like that in the result stack config:

aws:assumeRole:
  roleArn: arn/my-aws-arn

And simple key-value pairs keep working also.

Related issues (optional)

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

github-actions[bot] commented 2 months ago

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 commented 2 months ago

/run-acceptance-tests

github-actions[bot] commented 2 months ago

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

ljtfreitas commented 2 months ago

cc @rquitales @EronWright

blampe commented 2 months ago

This is awesome, thank you for the contribution! One initial thought -- still need to take a deeper look.

The idea is to enable people to just put their configs from Pulumi..yaml files in a ConfigMap, and the operator will be able to handle it.

This is a great idea. What do you think about using Pulumi's native marshalers? That would give you a little validation on top, and it might also make your flattening logic unnecessary (although I'm not 100%). The encoding package should do the trick, and you would unmarshal into a ProjectStack.

ljtfreitas commented 2 months ago

@blampe Thanks. About your suggestion maybe I haven't expressed myself properly. My initial idea, and that's the implementation is actually doing, is to put in a ConfigMap just the config node from a Pulumi.<stack>.yaml file, not the whole ProjectStack. Pulumi.<stack>.yaml can contain fields like secretsprovider, as the struct that you mentioned have, but it was not my idea to be able to configure them through a ConfigMap.

However, it looks like a good idea to me 🤔 ; so we could move the whole stack file (Pulumi.<stack>.yaml) to a ConfigMap and we even wouldn't have to configure things like secretsprovider or backend in the CRD 🤔 ; we could just unmarshal the ConfigMap key's content as a ProjectStack. Not sure if it's related with the specific topic of structured config but looks good to me.

blampe commented 2 months ago

@blampe Thanks. About your suggestion maybe I haven't expressed myself properly. My initial idea, and that's the implementation is actually doing, is to put in a ConfigMap just the config node from a Pulumi.<stack>.yaml file, not the whole ProjectStack. Pulumi.<stack>.yaml can contain fields like secretsprovider, as the struct that you mentioned have, but it was not my idea to be able to configure them through a ConfigMap.

However, it looks like a good idea to me 🤔 ; so we could move the whole stack file (Pulumi.<stack>.yaml) to a ConfigMap and we even wouldn't have to configure things like secretsprovider or backend in the CRD 🤔 ; we could just unmarshal the ConfigMap key's content as a ProjectStack. Not sure if it's related with the specific topic of structured config but looks good to me.

Ah, I had misunderstood what all was in the ConfigMap, I didn't realize it was just the config key. In that case you could marshal the config.Map instead of the whole ProjectStack as I had suggested earlier. I didn't mean to increase the scope of your change!

ljtfreitas commented 2 months ago

@blampe Don't worry, maybe I haven't expressed myself properly in the PR description (English is not my first language so sometimes talking about complex stuff is just...complex 😅). About your suggestion, I'm trying to refactor the code to use config.Map but I'm struggling to read the content. When I try to unmarshal I see this error:

could not parse <my-config> as a configuration key (configuration keys should be of the form `<namespace>:<name>`

Looking at the code that deserializes a YAML to a ProjectStack instance (in the Automation API code), config keys are namespaced (with the project name) before the config.Map's unmarshal 😕 ...and I can't use the same function because it's private. So config keys must be namespaced in the ConfigMap.

Not sure if we need to enforce that here because this behavior will be applied anyway during the config-set phase (and speaking for myself I don't namespace my configs because of this)

ljtfreitas commented 2 months ago

hey friends, any news here? we really need this feature

@blampe @rquitales

blampe commented 2 months ago

Sorry for the delay @ljtfreitas, I plan on playing around with this tomorrow!

ljtfreitas commented 1 month ago

thank you @blampe, don't worry, don't need to apologize, thank you for your support (i hope that I have not sounded like a jerk or something 😅 also), i will check the comments 👀

github-actions[bot] commented 1 month ago

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 commented 1 month ago

hi friends 👋 , any news here? cc @blampe

miguelslemos commented 1 month ago

Hi @blampe, this feature is crucial for us, could you take a look when you get time, please?

github-actions[bot] commented 1 month ago

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 commented 1 month ago

hey @blampe , I've made some changes and updated the PR description; I've removed the structured concept and started to handle just as literal values, both simple or complex ones.