Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
NET-581 - Added vault namespace in helm #2841
NET-581 - Added vault namespace in helm #2841
Changes from all commits
bc79b0a
3ee878c
10ec942
9935a1b
4097b48
3f8c4f3
5339dd9
c734284
9e2f11d
9d2ba12
8cc2117
01f3e95
6917abb
ad710bf
fdd7cf1
d3e0323
7b3482b
532210d
fd469b6
7c870ea
cac8d3a
c4a0f8a
6c692df
d05f340
261f379
3d9c05c
228b423
ef1710d
0827f81
306632b
faec4c2
b1fd0fd
636208f
666331a
ffe4b1f
c8478a0
ecfff91
959431f
8f52b7c
1f3e23a
ed1026e
825fb42
b840d32
ed2573a
a47a3d8
8a7b8bd
3c4291c
4523282
af7e617
124cc4d
4be3cea
7c13074
d6da52e
5f6a200
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json structure is like this https://github.com/hashicorp/consul-k8s/pull/2841/files#diff-73787600ab1d7b64c9a865fad8d8520d230644c998b3bf9b5670ffdb8900bb2eR713.
was having trouble fetching keys from nested array of json. hence had to keep it as string.
@zalimeni
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@absolutelightning good catch, my example was off - but I think this is still possible, see this updated version.
From the link above, if you use:
does that work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the correct syntax.
See -
consul-k8s/charts/consul/values.yaml
Lines 234 to 242 in 8f52b7c
its inside json array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@absolutelightning ah, I'm sorry - that was unintuitive to me and I missed the
[]
s this whole time. (I also suspect looking at Consul code on the server side that it's not strictly necessary and this is an example of our JSON->HCL parsing complexity... 😞)This looks overly complicated to detect in Helm template syntax even w/ the
first
template function as a sort-of option, so I feel a"namespace"
contains
check is a reasonable trade-off, given how unlikely it is someone would use that as a value (which would cause us to skip thevaultNamespace
addition).cc @thisisnotashwin in case you have any other thoughts, since we discussed this before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good! JSON parsing is a pain 😞
💯