LOG-7804: support custom headers for Elasticsearch output #3073
Conversation
openshift-ci
bot
added
the
needs-ok-to-test
|
Hi @AndrewChubatiuk. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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-sigs/prow repository. |
|
/retitle NO-JIRA: support custom headers for Loki and Elasticsearch outputs Not sure if this needs a JIRA ticket (probably it does, as there should be a release note and we'll want to backport this too) |
openshift-ci
bot
changed the title
openshift-ci
bot
added
ok-to-test
openshift-ci-robot
added
the
jira/valid-reference
|
@AndrewChubatiuk: This pull request explicitly references no 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 openshift-eng/jira-lifecycle-plugin repository. |
AndrewChubatiuk
changed the title
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
@AndrewChubatiuk if the intent is to use this to write to VictoriaLogs then is there a functional test we can provide to confirm functionallity. The larger question is what happens when RH logging to VictoriaLogs doesn't work and we are on the hook to support it with no backing tests. |
AndrewChubatiuk
force-pushed
the
support-custom-headers-for-loki-and-elasticsearch-outputs
branch
from
981242e to
71338b8
Compare
|
/retest |
AndrewChubatiuk
force-pushed
the
support-custom-headers-for-loki-and-elasticsearch-outputs
branch
7 times, most recently
from
d3b60ae to
760d646
Compare
|
@jcantrill added functional test with custom headers |
jcantrill
changed the title
|
@AndrewChubatiuk: This pull request references LOG-7804 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.8.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
hey @jcantrill |
The only question I have is if we should carte blanche allow any header and my gut says "no". The two headers in question is "Authorization" and "Content-Type". Elasticsearch only accepts 'ndjson' . The other concern is authorization which is dictated by other settings in the API. I am of the opinion there should be a validation which generates an error and condition when they exist in the map |
vector sets authorization header after custom headers, it overrides one if it's set using custom headers |
|
Good point on these two headers - we should update the test to make sure Authorization and Content-Type cannot be overridden via custom headers |
AndrewChubatiuk
force-pushed
the
support-custom-headers-for-loki-and-elasticsearch-outputs
branch
from
760d646 to
cb3bd02
Compare
|
@jcantrill @vrutkovs added validation for headers |
AndrewChubatiuk
force-pushed
the
support-custom-headers-for-loki-and-elasticsearch-outputs
branch
from
cb3bd02 to
1c2c123
Compare
|
@AndrewChubatiuk this LGTM. Please squash your commits and update the comment to reflect the title. Thank you for your contribution |
AndrewChubatiuk
force-pushed
the
support-custom-headers-for-loki-and-elasticsearch-outputs
branch
from
1c2c123 to
8db49a9
Compare
|
@AndrewChubatiuk: This pull request references LOG-7804 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.8.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@AndrewChubatiuk: This pull request references LOG-7804 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.8.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@jcantrill done |
AndrewChubatiuk
force-pushed
the
support-custom-headers-for-loki-and-elasticsearch-outputs
branch
2 times, most recently
from
df7fdae to
05eea16
Compare
AndrewChubatiuk
force-pushed
the
support-custom-headers-for-loki-and-elasticsearch-outputs
branch
from
05eea16 to
b7bd0a5
Compare
|
@AndrewChubatiuk: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
@AndrewChubatiuk: This pull request references LOG-7804 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.8.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@AndrewChubatiuk: This pull request references LOG-7804 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.8.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/approve |
openshift-ci
bot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AndrewChubatiuk, 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 |
|
/hold |
openshift-ci
bot
added
approved
6 participants
Description
This PR adds ability to set custom headers for Elasticsearch output to allow data ingestion from Openshift into VictoriaLogs
Links
https://issues.redhat.com/browse/LOG-7804
/cc @Clee2691 @cahartma @jcantrill
/assign @jcantrill