-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
filter_kubernetes: add option kube_token_ttl (#4352) #4487
Conversation
The option sets the re-read frequency of the token for the defauld method and for option Kube_Token_Command. Default is 600 seconds. Signed-off-by: Michael Voelker <[email protected]>
Tested with config:
Logoutput:
Valgrind
|
Ignore the ActionLint failure, this is resolved on |
@novegit In my understanding, this patch will change below points.
Is the point 2 breaking change? It may be pointless question since I'm not familiar with k8s. |
For using 'Kube_Token_Command' to fetch the authorization token, there is no change, because there was "a reload token" with 600s frequency, but it can configured now to other values. |
Yeah I think this is perfectly acceptable - presumably you could set a longer interval as well. We must be assuming the best practices are followed for regular credential rotation. |
@novegit FYI: we are going to do the cut shortly, there are some pending changes |
Signed-off-by: Michael Voelker <[email protected]>
@edsiper whats missing? |
@edsiper looks like this got missed |
@edsiper This change is required for fluentbit running in EKS 1.21 clusters. I'm interested in getting this in. Please do let us know what else the PR requires to get the change in. |
{ | ||
FLB_CONFIG_MAP_INT, "kube_token_ttl", "600", | ||
0, FLB_TRUE, offsetof(struct flb_kube, kube_token_ttl), | ||
"kubelet token ttl" |
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 description should tell the unit- is this seconds or minutes or hours?
@patrick-stephens @edsiper Can we get this one merged and released? This is needed by AWS EKS customers. CC @lubingfeng |
I'm afraid I have no super powers for this one: CI is signed off though so I'll check with @edsiper |
@@ -847,6 +847,11 @@ static struct flb_config_map config_map[] = { | |||
0, FLB_TRUE, offsetof(struct flb_kube, kubelet_port), | |||
"kubelet port to connect with when using kubelet" | |||
}, | |||
{ | |||
FLB_CONFIG_MAP_INT, "kube_token_ttl", "60", |
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.
time based properties must use FLB_CONFIG_MAP_TIME type
if the original timeout was 10 minutes, now you are defaulting to 1 minute (60 seconds). That's a breaking change, it should keep the old defaults
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.
Ping
@@ -847,6 +847,11 @@ static struct flb_config_map config_map[] = { | |||
0, FLB_TRUE, offsetof(struct flb_kube, kubelet_port), | |||
"kubelet port to connect with when using kubelet" | |||
}, | |||
{ | |||
FLB_CONFIG_MAP_INT, "kube_token_ttl", "600", |
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 should be FLB_CONFIG_MAP_TIME
, then you can use 10m
as the default
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.
If we do this, the field on the struct should be uint64_t
?
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.
Never mind, flb_utils_time_to_seconds
returns an int. (which I think means the time config map type won't work well on systems where an integer is smaller than 32 bits, since a 16 bit integer can only hold enough seconds for about 1 day if I did my math right)
@novegit If you make the simple change as shown in this commit we can merge this PR: PettitWesley@0527e1a |
Signed-off-by: Michael Voelker <[email protected]>
sorry, its not really clear for me what to use now. In the last commit was FLB_CONFIG_MAP_INT / "600" => 10m . So I changed it now to FLB_CONFIG_MAP_TIME / "10m" |
@novegit This looks correct/as requested to me now. Thanks! |
@edsiper is there any chance of getting this merged ASAP? |
Thanks |
Should this PR have closed #5445? |
@stevehipwell may be yes but also its in many ways ideal to not close the issue until its released, so customers have something to search and find... of course github makes it easier to auto-close on merge so in general I think this repo uses that most of the time.. |
* filter_kubernetes: add option kube_token_ttl The option sets the re-read frequency of the token for the defauld method and for option Kube_Token_Command. Default is 600 seconds. Signed-off-by: Michael Voelker <[email protected]> * filter_kubernetes: set kube_token_ttl default to 600s Signed-off-by: Michael Voelker <[email protected]> * filter_kubernetes: use FLB_CONFIG_MAP_TIME for kube_token_ttl config Signed-off-by: Michael Voelker <[email protected]> Signed-off-by: Manal Geries <[email protected]>
* filter_kubernetes: add option kube_token_ttl The option sets the re-read frequency of the token for the defauld method and for option Kube_Token_Command. Default is 600 seconds. Signed-off-by: Michael Voelker <[email protected]> * filter_kubernetes: set kube_token_ttl default to 600s Signed-off-by: Michael Voelker <[email protected]> * filter_kubernetes: use FLB_CONFIG_MAP_TIME for kube_token_ttl config Signed-off-by: Michael Voelker <[email protected]> Signed-off-by: a445943 <[email protected]>
The option sets the re-read frequency of the token for the
"read serviceaccount file" method and for option Kube_Token_Command. Default is 600 seconds.
Before this change, the token was reloaded only for the Kube_Token_Command with fixed default value 600s.
Signed-off-by: Michael Voelker [email protected]
Enter
[N/A]
in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Documentation
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.