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

Support defining ClusterLogForwarding filter list as an object #155

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

HappyTetrahedron
Copy link
Contributor

I need to define logforwarding filters in multiple places in the hierarchy, and so would like to have a nice easily-mergeable dict instead of the list - similar to how we define the pipelines, inputs and outputs as well.

I built this feature in a backwards-compatible way - if filters are already defined as a list, they are kept as-is.

Of course, it is only possible to use either lists or objects within the entire hieararchy of a single cluster - if they are mixed, reclass will fail. Still, this allows us to migrate "legacy" filter lists as we find them.

Alternatively, I could change the implementation to be analogous to the other fields where we perform this kind of processing. Then we would have to update all existing filter definitions in our hierarchy, as it'd be a breaking change. I only know of the filters for loki central audit logging; I don't know in how many other places this is used.

Checklist

  • The PR has a meaningful title. It will be used to auto-generate the
    changelog.
    The PR has a meaningful description that sums up the change. It will be
    linked in the changelog.
  • PR contains a single logical change (to build a better changelog).
  • Update the documentation.
  • Categorize the PR by adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog.

@HappyTetrahedron HappyTetrahedron added the enhancement New feature or request label Oct 30, 2024
@HappyTetrahedron HappyTetrahedron requested a review from a team as a code owner October 30, 2024 15:36
@HappyTetrahedron
Copy link
Contributor Author

Documentation is TBD. I would like to hear your opinions on whether this is preferable or whether it's better to go with the breaking change.

@bastjan
Copy link
Contributor

bastjan commented Nov 1, 2024

Some other components use filters: [] + map(_filters: {}) which I personally think is the way to go.

I don't know tho if that fits the component here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants