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
LOG-4852: Vector collector Pods no longer picks up the log collector SAs Secret as a fallback #2284
Conversation
@Clee2691: This pull request references LOG-4852 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cherry-pick master |
@Clee2691: once the present PR merges, I will cherry-pick it on top of master in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Clee2691, jcantrill The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
/hold |
} | ||
} else if secret != nil { | ||
// Use secret of logcollector service account as backup | ||
tlsConf := security.TLSConf{ |
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 the logic is missing something here because it's not using any of the helper functions.
- What if my secret defines a CA to use?
- What if I define a secret ... but I don't get any TLS Profile options here
Following "GenerateTLSConf" leads me to think it still nees to utilize that function... and then check the CA afterwards and set it like this if not defined
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.
The first if statement is looking for output.secret
so if a user defines a secret for the output, it will be caught there. Your questions above pertain to that bit.
Line 221
if o.Secret != nil {
...
}
I'm not changing any of the original functionality as this is only using the logcollector
secret as a fallback if a user does not define a secret for the output. The secret
here isn't the one that is defined for the output. It is the secret automatically populated for the legacy case (constants.LogCollectorToken
).
…SAs Secret as a fallback
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.
/hold cancel
/lgtm |
920d952
into
openshift:release-5.8
@Clee2691: #2284 failed to apply on top of branch "master":
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Description
This PR addresses forwarding to an internal
LokiStack
where in the legacy case, if thepipelines.outputs
is not default it will not apply thelogcollector-token
secret'stoken
andca.crt
. For a CLF named instance it will add thelogcollector
service account token to be used./cc @cahartma @vparfonov
/assign @jcantrill
Links