Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expanding a reference into an object #115

Open
kroonprins opened this issue Dec 20, 2022 · 2 comments
Open

expanding a reference into an object #115

kroonprins opened this issue Dec 20, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@kroonprins
Copy link

hi, I am using vals and was thinking it could be useful if we were to be able to do something like this

apiVersion: v1
kind: Secret
metadata:
  name: my-secret
stringData: ref+azurekeyvault://my-vault#/* 
type: Opaque

to resolve to:

apiVersion: v1
kind: Secret
metadata:
  name: my-secret
stringData:
  secret1: value1
  secret2: value2
  ... all secrets in the vault
type: Opaque

So basically something to avoid having to list every secret separately when I want them all.

In a more general way something like:

# file: test.yml
a: b
c:
  d: e
$ cat <<EOF | vals eval -f -
# existing behavior of vals:
string: ref+file://test.yml
one_field: ref+file://test.yml#/c/d
# added functionality:
expanded: ref+file://test.yml#/*
partly_expanded: ref+file://test.yml#/c/*
EOF

Gives:

string: |-
  a: b
  c:
    d: e
one_field: e
expanded:
  a: b
  c:
    d: e
partly_expanded:
  d: e

I did what I think are the required changes for that in this fork: https://github.com/kroonprins/vals, but the change is not super small, so it is not without risk of breaking things. So if you think the use case is not useful enough, or not in line with the idea behind vals, or too risky, no problem, I can keep using the fork, but in case you do think it could be useful for others I could turn it into a PR.

@mumoshu
Copy link
Collaborator

mumoshu commented Feb 4, 2023

@kroonprins Hey! Thanks you so much for the detailed proposal and your awesome work. This is indeed something that I wanted to include into vals.

To sync up with you, I wanted a syntax that:

  1. Almost conforms to the schema/syntax checker of the original YAML document so that the linters and checkers for the original config file can still be applied to the "vals"-ified config file.
  • In your case, that's core/v1 Secret data, whose value is supposed to be key-value pairs of strings.
  1. Could support/or can be enhanced later to support two or more objects being merged in a specific and deterministic order.
  • With your example, you can merge in an object only once per the key/object(= Secret's data in your case)
  1. Could support both (1)merging in the json/yaml document embedded in a single secret value and (2)merging in multiple secrets under the specified path as key-value pairs

That said, it would be fantastic if you could help me figure out what's the best syntax that accommodates the three points.

My first idea would be:

apiVersion: v1
kind: Secret
metadata:
  name: my-secret
stringData:
  01-commonazurekvsecrets objref+azurekeyvault://common-vault#/*: whatever
  02-specificsecrets objref ref+azurekeyvault://my-vault#/*: blah
type: Opaque

Note that this isn't tested and I'm unsure if this actually conforms to the K8s API, especially on a complex key like "01-commonazurekvsecrets objref+azurekeyvault://common-vault#/*".

My motivation here is to let the vals expression appear within the key so that the user and vals can differentiate it from the normal case where the vals expression appears in the value.

I've also used "objref+" over our standard "ref+" so that it's even more clearer that it is something that is different from the normal "ref+" expression.

Lastly, the ability to include any prefixes before the objref+ would be handy for sorting those keys before merging, so that my requirement 3. can be addressed.

WDYT? Do you have any other good idea(s)?

@mumoshu mumoshu added the enhancement New feature or request label Feb 4, 2023
@kroonprins
Copy link
Author

hello @mumoshu ,

Interesting points.

Making it more explicit by introducing an "objref+" seems indeed a good idea. From a user perspective I think that would already be sufficient to differentiate it from the normal case?

For k8s, keys are expected to conform to '[-._a-zA-Z0-9]+' (not even spaces), so having a vals ref as a key would be problematic indeed for requirement 1. Also what irks me slightly in the proposal is the need for a "whatever" and a "blah" 😄. So somehow getting the vals ref back to the right of the ":" would be nice, but I must admit I don't immediately see a good syntax that then supports the 3 requirements. You would need something in the ref to indicate whether you want to be expanded under the key or get merged in the parent context. Perhaps you can have an "objref+" and a "hoistedobjref+" (or a better name), but it doesn't sound too appealing either. And this would also not solve the sorting before merging for a situation like this:

stringData:
  01-commonazurekvsecrets: hoistedobjref+azurekeyvault://common-vault#/*
  02-specificsecrets: hoistedobjref+azurekeyvault://my-vault#/*
  my-secret-i-want-to-override: ref+azurekeyvault://my-even-more-specific-vault/my-secret

Just for info: what I had in the fork, did somehow support requirement 2, by combining multiple references with a "+" in between (like is already possible in vals currently for normal refs), e.g.

key: ref+file://file1.yml#/*+ref+file://file2.yml#/*

The order in the merging was then the order of the references. But this is also limited (e.g. a situation where you have objref+azurekeyvault://common-vault#/* from which you would want to override one specific secret with a ref+azurekeyvault://my-vault/my-secret).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants