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

[FEATURE] Separation of MIB definitions and auth info #221

Closed
candlerb opened this issue Aug 16, 2017 · 25 comments
Closed

[FEATURE] Separation of MIB definitions and auth info #221

candlerb opened this issue Aug 16, 2017 · 25 comments

Comments

@candlerb
Copy link
Contributor

Currently a named "module" in snmp.yml defines the MIBs to walk / extraction of metrics, plus the authentication parameters to connect to a device.

I would like to separate out the authentication parameters, so that I could scrape e.g. [^1]:

/snmp?target=1.2.3.4&module=apcups&auth=London1
/snmp?target=5.6.7.8&module=apcups&auth=NewYork1

This means that if I have multiple instances of the same type of device but using different secrets, I don't need to duplicate the module definition. And conversely, if I'm using the same secret across multiple types of device, I don't need to modify every module definition just to insert the same secret.

One approach is to have a separate file auth.yml like this:

London1:
  version: 2
  auth:
    community: FooBar
NewYork1:
  version: 2
  auth:
    community: HotDog

That has a number of benefits:

  • no change to the existing snmp.yml format
  • auth.yml could have more restrictive permissions
  • allows snmp.yml to be created easily by concatenating a bunch of static, pre-built module definitions [^2]

However, I am also amenable to snmp.yml having separate modules and auth sections if that's preferred. I do note that generator.yml already has a top-level modules section.


[^1] I realise I still have to define a separate scrape job for all targets which share the same params. It would be nice to be able to override params on a per-target basis, but that's a separate issue.

[^2] And these can be published by anyone who wants to, à la Nagios Plugin Exchange

@brian-brazil
Copy link
Contributor

URL parameters are not considered secure in the Prometheus security model, so that is not an appropriate solution.

What I'd suggest is putting placeholders in your generator.yml, and then getting your configuration management system to replace them in the snmy.yml.

@candlerb
Copy link
Contributor Author

URL parameters are not considered secure in the Prometheus security model

Sure, but I don't see how that applies here. I'm not proposing passing any secrets in the URL parameter; only a label such as "auth=London1" which references a named bundle of values in the auth config, that contains the actual auth config.

It is true that if the snmp_exporter is visible to me, I can send a request for

/snmp?target=<my-ip-address>&module=apcups

and this will trigger an outbound SNMP query to my IP address containing the credentials which I can easily capture (apart from SNMPv3). But this is a security problem with snmp_exporter today, and what I propose makes no difference to this.

What I'd suggest is putting placeholders in your generator.yml, and then getting your configuration management system to replace them in the snmy.yml.

This doesn't conveniently address the case where I have two or more groups of devices which require the same MIBs, but different credentials.

I would have to replicate the snmp.yml section N times, with N different sets of creds, and generate N different module names.

If I have M modules and N sets of creds, I could pre-generate MxN modules with names like <module>/<auth>, e.g. default/London1, apcups/NewYork1 etc. Ugly but would work, as long as M is reasonably low.

But I'd also like to get to the point where I could publish snmp.yml modules for different types of devices, and people could use them without having to modify them (specifically without having to turn them into templates which are expanded N times)

@brian-brazil
Copy link
Contributor

If I have M modules and N sets of creds, I could pre-generate MxN modules with names like /, e.g. default/London1, apcups/NewYork1 etc. Ugly but would work, as long as M is reasonably low.

Yes, that's how to do it.

But I'd also like to get to the point where I could publish snmp.yml modules for different types of devices, and people could use them without having to modify them (specifically without having to turn them into templates which are expanded N times)

That's something I encourage, though you're going to always need to customise it for auth.

@candlerb
Copy link
Contributor Author

That's something I encourage, though you're going to always need to customise it for auth.

If the auth were separated out, customisation would no longer be required.

I could just cat together some published files to make snmp.yml:

# cat cisco4xxx.yml netgear73xx.yml >snmp.yml
cisco4xxx:
  walk: ...
  metrics: ...
netgear73xx:
  walk: ...
  metrics: ...

Define my auth setting(s):

# vi auth.yml
site_london:
  version: 2
  auth:
    community: wibble

And scrape:

  - job_name: snmp_cisco4xxx_london
    static_configs:
      - targets: ...
    metrics_path: /snmp
    params:
      module: [cisco4xxx]
      auth: [site_london]

  - job_name: snmp_netgear73xx_london
    static_configs:
      - targets: ...
    metrics_path: /snmp
    params:
      module: [netgear73xx]
      auth: [site_london]

@brian-brazil
Copy link
Contributor

This is not something we're likely to add. It's presumed you have a configuration management system to deal with this.

@candlerb
Copy link
Contributor Author

Amenable to contrib, or not?

@brian-brazil
Copy link
Contributor

No, this would complicate the configuration for simple uses cases and make configuration management more complicated for the typical user. Our configuration files are meant to be quite dumb.

@SuperQ
Copy link
Member

SuperQ commented Aug 17, 2017

I'm on the fence about this. I know it's just an example, but if you have a location-dependent auth, it would be best to have a snmp_exporter dedicated in that location. SNMP is very chatty, and UDP. We see lots of problems trying to send SNMP packets cross-datacenter.

But, I can see the appeal of separating the auth from the walk. The difficulty is we want "one" good way to do things.

@candlerb
Copy link
Contributor Author

"One good way" makes sense!

It does seem to me that "which MIBs to walk" and "which auth creds to use" are naturally orthogonal. One is the property of the device type, as built by the manufacturer, and is the same for all such devices; the other is a property of the actual device instance(s) you own, as per the way you have configured them.

Also, "which MIBs to walk" is potentially composable:

/snmp?target=x.x.x.x&module=default&module=ciscoenv&auth=London1

params:
  module: [default, ciscoenv]
  auth: [London1]

Anyway, I'll just outline my specific use case, which is to build a front-end to Prometheus which acts as a simple but scalable Cacti/LibreNMS replacement.

I would of course expect that adding a new type of device would require creating/installing new snmp.yml entries (and similarly new alerting rules etc).

Adding a new instance of a device is something that would normally happen just via target scraping config, which in turn could be picked up from file_sd_config or consul_sd_config or whatever. Ideally this wouldn't need to touch snmp.yml or restart snmp_exporter.

So when the user goes to the UI and says "add device x.x.x.x, poll it using module Foo and auth type Bar", the front-end just adds x.x.x.x to the snmp_Foo_Bar scrape job (or creates a new scrape job, if this is the first device like that)

I do realise that I need to create the snmp_Foo_Bar scrape jobs (*), and I could therefore generate the Foo_Bar snmp modules at the same time. It involves injecting the creds into the right places in the yaml, concatenating the modules, restarting snmp_exporter etc. This is of course doable, although more tricky if snmp_exporter is remote.

I just thought that the SOC was a good thing to do in any case.


(*) because there are no per-target overrides of params in prometheus.

Observation: the job name is part of the time series definition. So if you change the community string on a device, and move it from scrape job snmp_cisco_London1 to snmp_cisco_London2, then this will create new time series for all its metrics. It should be possible to force the job name to snmp_cisco via relabelling though.

@SuperQ
Copy link
Member

SuperQ commented Aug 17, 2017

It's not in the default example, but you can override the params on a per-target basis with regexp matching in the rewrite_configs. This would allow for one large "snmp" job. But you would want to have additional labels like device_type="module" in order to aggregate.

@candlerb
Copy link
Contributor Author

Do you mean relabel_configs?

Google for "prometheus rewrite_configs" doesn't turn up anything, and the string rewrite_config does not appear in the source.

Is it possible to change the label __param_<name> and have it reflect in the generated URL?

@candlerb
Copy link
Contributor Author

Here is another suggestion: what about being able to PUT or POST to /config to reconfigure snmp_exporter?

I'd still need to expand/merge templates, but at least there would be an easy way to deploy them. Authorisation potentially an issue, but see #225.

@brian-brazil
Copy link
Contributor

brian-brazil commented Aug 17, 2017

It does seem to me that "which MIBs to walk" and "which auth creds to use" are naturally orthogonal

Not quite. Noone has requested it yet, but some devices let you select VLAN via the community strings.

Here is another suggestion: what about being able to PUT or POST to /config to reconfigure snmp_exporter?

Our one way of doing this is via a file. You can setup something yourself to permit this.

@SuperQ
Copy link
Member

SuperQ commented Aug 17, 2017

Sorry, yes, I meant relabel_configs. Yes, you can use __param_<label>. For example, you could have targets named "foo:cisco" and then extract into __param_module.

    relabel_configs:
      - source_labels: [__address__]
        regex: "(.*):(.*)"
        replacement: ${2}
        target_label: __param_module
     - source_labels: [__address__]
        regex: "(.*):(.*)"
        replacement: ${1}
        target_label: __param_target
      - source_labels: [__param_target]
        regex: "(.*):(.*)"
        replacement: ${1}
        target_label: instance

@SuperQ
Copy link
Member

SuperQ commented Aug 18, 2017

I have seen people want to split auth from walk previously, to reduce the duplication and size of the configuration yaml. I think this is a good idea.

CC: @RichiH @fjaeckel

@brian-brazil
Copy link
Contributor

The size of the configuration file is not important, it's autogenerated.

I'm against making this more complicated, it's a matter for configuration management.

@fjaeckel
Copy link

As the configuration is autogenerated, I would not necessarily separate the authentication from the main configuration as it adds another file one would need to manage and generate.
Ultimately it doesn't make a difference in my opinion, however if you find more traditional environments, that do not understand the concepts of Vault - one might be able to have a "authentication" file.

In my specific scenario, we don't have many different communities or authentication ways, as it is already walled of by ACLs.

@SuperQ
Copy link
Member

SuperQ commented Aug 18, 2017

My thought was not an additional file, but a separate section in the same file.

@brian-brazil
Copy link
Contributor

I prefer to avoid indirection like that, it makes the config harder to read and create for a human. Also most environments will not need it, as it'll be at most one set of auth per device type.

@brian-brazil
Copy link
Contributor

If this is something you want you're free to build tooling on top of the snmp exporter/generator, but I intend to keep things simple in order to keep both typical use and development of such tooling easy.

@RichiH
Copy link
Member

RichiH commented Aug 21, 2017

Noone has requested it yet

JFTR, this has been requested in the past. Now that generator will include a way to do multiple lookups soon(tm), I foresee that the use case of "can you send me your manual config as a paste" will fall off dramatically.

@candlerb
Copy link
Contributor Author

candlerb commented Sep 9, 2017

It occurs to me that there is another approach: merging YAML references using the << merge key (spec).

This is an optional part of YAML so I don't know if the go YAML parser supports it.

It looks something like this:

apcups: &apcups
  walk:
  - 1.3.6.1.2.1.1.3
  - etc
apcups_london:
  <<: *apcups
  version: 2
  auth:
    community: blah

Using Python's yaml parser I get:

>>> x = yaml.load(open("/home/brian/tmp.yml"))
>>> x
{'apcups_london': {'version': 2, 'auth': {'community': 'blah'}, 'walk': ['1.3.6.1.2.1.1.3', 'etc']}, 'apcups': {'walk': ['1.3.6.1.2.1.1.3', 'etc']}}

Conveniently, the references point to the same underlying object, so the storage is shared.

>>> id(x['apcups_london']['walk'])
140041579271160
>>> id(x['apcups']['walk'])
140041579271160

@SuperQ
Copy link
Member

SuperQ commented Sep 9, 2017

Yes, you're right, yaml merging is a good option. I tested this with the snmp_exporter's yaml parser, works correctly.

@Gelob
Copy link

Gelob commented Jul 23, 2019

No, this would complicate the configuration for simple uses cases and make configuration management more complicated for the typical user. Our configuration files are meant to be quite dumb.

@brian-brazil @SuperQ
I'm not sure how its easy now because out of the box if you are like anyone and change your snmp community from public to something else you have to go into snmp.yml and add an auth: block. not even editing an existing one and changing a string but adding in a whole new block, but if I want to do it the recommended way now I have to go get and build the generator and do it with that. It felt to me that if I didn't use public as my SNMP community starting to use snmp_exporter is a giant chore.

@candlerb
Copy link
Contributor Author

Cross-ref: this is now going forward in issue #619 and PR #859. Thank you!

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

No branches or pull requests

6 participants