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 auth alongside target & module as distinct property #619

Closed
RichiH opened this issue Feb 25, 2021 · 5 comments · Fixed by #859
Closed

Support auth alongside target & module as distinct property #619

RichiH opened this issue Feb 25, 2021 · 5 comments · Fixed by #859

Comments

@RichiH
Copy link
Member

RichiH commented Feb 25, 2021

auth is orthogonal from the target, the data model and from what data to receive. As such, it will be split out into its own section.

For backwards compatibility, inline auth should be supported, but the new pattern encouraged.

Open questions:

  • Should we allow auth to live in a secondary file to completely detach global OID etc configuration from user-specific config?
  • Should we offer inline-line loading of files or explicitly have a few distinct files? Inline seems better.
  • Should we offer a baseline UI to show which files are loaded?
  • Should the exporter emit information like files loaded in _info or through a gauge?

As this touches a lot of questions in #85 CC @glensc

@RichiH RichiH changed the title Support auth alongside target & module Support auth alongside target & module as distinct property Feb 25, 2021
@RichiH
Copy link
Member Author

RichiH commented Feb 25, 2021

@RichiH
Copy link
Member Author

RichiH commented Mar 15, 2021

After more thought:

Should we allow auth to live in a secondary file to completely detach global OID etc configuration from user-specific config?
#85 will take care of this and people can structure it as they wish.

The new default config we provide should most likely break it out.

Should we offer inline-line loading of files or explicitly have a few distinct files? Inline seems better.

I flipped. Let's make it explicit on CLI. Directory support takes away the need for inlinig, IMO. As such, we should offer fewer ways to do the same thing to avoid long-term confusion.

Should we offer a baseline UI to show which files are loaded?

I think we must.

Should the exporter emit information like files loaded in _info or through a gauge?

I think we should. Unclear if we should do

  • one _info, lots of labels
  • one _info, overload a single label
  • one metric per file

New open question: Does that mean we're listing the directory name, what is found and activated inside, or both of those?

@xkilian
Copy link

xkilian commented Mar 15, 2021

What you are describing should be handled in logging at exporter startup, and in the web UI. It is not meant for info metrics IMO.

@pobk
Copy link

pobk commented Jul 25, 2022

My 2 pence:

  1. Credentials should be stored separately from the MIB definitions. Having a file which contains credentials separate from the snmp config would allow a greater flexibility around securing that file.
  2. Allowing the exporter to be targeted using credentials= as well as module= would allow for some flexibility in targeting.

My employer uses LastPass, and we've integrated that into an ansible workflow. This gives greater protection to our systems estate since if you don't have access to the credentials from the credentials store you cannot access the system to run any of the playbooks. This is something that could easily be implemented in other ways too.

This also works for generating files full of credentials where each node has separate credentials configured.

SuperQ added a commit that referenced this issue Mar 26, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Mar 26, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Mar 26, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Mar 26, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Mar 27, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Mar 27, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Apr 4, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Apr 5, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Apr 6, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Apr 6, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Apr 6, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Apr 6, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
@candlerb
Copy link
Contributor

I'm guessing that legacy files will still work?

modules:
  foo:
    auth: # is a legacy 'auth' section allowed here?
      ...

If not, or we want a clean break, it would be very easy to write a conversion tool which converts snmp.yml into the new form, providing an "auth" named the same as each original "module":

# output from tool
auths:
  foo:
    ...
modules:
  foo:
    ...

Then a scrape which supplies module but not auth could implicitly look for an auth with the same name (falling back to "public_v2" if that doesn't exist).

Another possibility would be to stick with the existing file format, but allow users to create modules which have only auth, only SNMP, or both:

modules:
  foo:
    walk:
      ...
    get:
      ...
  bar:
    auth:
      ...
  baz:
    auth:
      ...

Then you could do:

/snmp?module=foo&auth=bar    # use the 'foo' module but the 'auth' creds from bar
/snmp?module=foo&auth=baz    # use the 'foo' module but the 'auth' creds from baz
/snmp?module=foo             # use the 'foo' module with 'auth' creds under foo (if any): legacy mode

However, I think in the long run the top-level separation is cleaner.

SuperQ added a commit that referenced this issue May 3, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue May 3, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Jun 15, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Jun 17, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Jun 17, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Jun 17, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Jun 19, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: #619

Signed-off-by: SuperQ <[email protected]>
stephan-windischmann-sky pushed a commit to stephan-windischmann-sky/snmp_exporter that referenced this issue Oct 27, 2023
Allow configuration of auth/version parameters separately from the walk
and metrics in the generator and exporter configuration.
* Simplify startup with `ReloadConfig()`
* Make sure to init metrics on config reload.

Fixes: prometheus#619

Signed-off-by: SuperQ <[email protected]>
Signed-off-by: Stephan Windischmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants