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

Configuration checks breaks freeform keys #676

Closed
pieterlexis-tomtom opened this issue Apr 12, 2024 · 4 comments · Fixed by #678
Closed

Configuration checks breaks freeform keys #676

pieterlexis-tomtom opened this issue Apr 12, 2024 · 4 comments · Fixed by #678
Labels
bug Something isn't working
Milestone

Comments

@pieterlexis-tomtom
Copy link
Contributor

pieterlexis-tomtom commented Apr 12, 2024

Describe the bug
The Scalyr logger has 2 configuration settings (sessioninfo and attrs) that can contain arbitrary key-value pairs. This is extra information sent alongside the data. This is configured as follows:

    loggers:
    -   name: scalyr
        scalyrclient:
            apikey: XXXXX
            attrs:
                service: dnstap
                type: queries
            flush-interval: 10
            mode: flat-json
            sessioninfo:
                cloud_provider: Azure
                cloud_region: westeurope

When starting go-dnscollector, it errors (key name might differ, based on parsing order):

config error: unknown YAML key `cloud_region` in configuration

To Reproduce
Configure the scalyr logger as shown above, start go-dnscollector.

Expected behavior
go-dnscollector starts fine.

Additional context

  • Version 0.43.0 and commit 0267e3b

These fields in the config are a map[string]string and a map[string]interface{}.

https://github.com/dmachard/go-dnscollector/blob/0267e3b47aa374b51df84ceacb61238258d3fdea/pkgconfig/loggers.go#L239-L240

Perhaps these field types should be ignored when converting the config to a map before checking it?

@dmachard dmachard added the bug Something isn't working label Apr 12, 2024
@pieterlexis-tomtom
Copy link
Contributor Author

I had a look at the code and am not convinced this will be an easy fix, seem as how the keys are actually checked. It looks like the whole config checker might need a re-design :(

@dmachard
Copy link
Owner

I agree with you, the complexity of the config checker is too high, a complete redesign is needed. Any helps are welcome :)
In the meantime, I will push a quick and dirty fix :(

@dmachard
Copy link
Owner

FYI I have started a redesign of the config checker

@pieterlexis-tomtom
Copy link
Contributor Author

Cool! Can you explain the design? Perhaps I can chime in with ideas or code :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants