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
Implement k8s secrets provider for Agent #24789
Implement k8s secrets provider for Agent #24789
Changes from 13 commits
996850e
fa16391
0b582b3
a0ad168
bb4fc0c
a2d9051
5e1dad2
be9f7b1
898e9cb
c731628
5d20635
da4a3d8
68d3ea7
5db6cad
5a85afb
eea350a
f023688
72cccc7
ae81eb1
fb8a33e
2332aec
7a0bdcf
22b5857
7551ef5
e47bb5a
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.
Is it possible that getting a value from a fetch context provider returns something other than a string?
Below you can see that it will replace full objects if its a
varString
. I worry based on placing this before theswitch val.(type)
we are missing that as a possibility with a fetch context provider.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 see, however no strong opinion here since I'm not super familiar with the combinations that could occur. Do you think that removing
continue
and being able to get to theswitch
block after theFetch()
action would be sth that we want?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.
Now that I took a closer look at this file. This is definitely in the wrong place. At its current location it would try to find a constant string in a fetch context provider, we do not want that.
Looking at it, this needs to be removed from here and moved into the:
Leaving this function untouched.
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.
@blakerouse I tried to move (see f023688) the code inside
func (v *Vars) Lookup(name string) (interface{}, bool)
method of the same file but it does not seem to work (using same configuration so far) and I'm not sure that I can follow the flow here. I see that the method is called atbeats/x-pack/elastic-agent/pkg/eql/visitor.go
Line 281 in 01eb297
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 I see the issue. You need to do the following:
Add the following function to
Vars
:Then change
node, ok := Lookup(v.tree, val.Value())
tonode, ok := v.lookupNode(val.Value())
.See if that works.
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 fixes the issue, thank you!