Skip to content
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-2864: Compatibility with ES default output #1587

Merged

Conversation

xperimental
Copy link
Contributor

@xperimental xperimental commented Aug 3, 2022

Description

Currently the LokiStack integration always creates two outputs, one for each of the log streams that is forwarded by default (application and infrastructure). While these outputs are called default and default-infra, they do not behave like the default output that is available when using the ElasticSearch log storage.

This PR changes the behaviour of the default output reference when using LokiStack, so that it is internally split into different outputs depending on the log stream forwarded into it. This should ensure that an input is always forwarded into the correct tenant, thus enforcing the correct authorization. Additionally this enables the use of custom ClusterLogForwarder configurations that were usable with the ElasticSearch log storage, for example to forward audit logs into the LokiStack log store.

Known limitations:

  • The ClusterRole assigned to the logcollector currently always has write access to all tenants not just the ones it needs for the current configuration.

/cc @cahartma
/assign @jcantrill

Links

@xperimental
Copy link
Contributor Author

/retest

@jcantrill
Copy link
Contributor

/approve
/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 3, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcantrill, xperimental

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2022
@xperimental xperimental force-pushed the lokistack-default-compatibility branch from e98fb6d to 638bd29 Compare August 3, 2022 12:24
@xperimental
Copy link
Contributor Author

/retest

1 similar comment
@xperimental
Copy link
Contributor Author

/retest

Copy link
Contributor

@cahartma cahartma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2022
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 2 against base HEAD d0e2f85 and 8 for PR HEAD 638bd29 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 1 against base HEAD d0e2f85 and 7 for PR HEAD 638bd29 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD d0e2f85 and 6 for PR HEAD 638bd29 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 4, 2022

@xperimental: 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/test-infra repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants