inlet/snmp: do not deprecate default-community

We did not handle all cases, notably the case where default-community
was not set explicitely by the user. This seems a lot of code for
little gain, let's keep things simple.
This commit is contained in:
Vincent Bernat
2022-08-01 07:15:50 +02:00
parent 1bbaf4de24
commit e5d3d3fbc3
8 changed files with 13 additions and 141 deletions

View File

@@ -24,7 +24,7 @@ type SubnetMap[V any] struct {
// Lookup will search for the most specific subnet matching the
// provided IP address and return the value associated with it.
func (sm *SubnetMap[V]) Lookup(ip net.IP) (V, bool) {
if sm.tree == nil {
if sm == nil || sm.tree == nil {
var value V
return value, false
}

View File

@@ -40,8 +40,8 @@ documentation.
- `clickhouse``asns` to give names to your internal AS numbers
- `clickhouse``networks` to attach attributes to your networks
- `inlet``snmp``communities` to set the communities to use for
SNMP queries
- `inlet``snmp``default-community` to set the default community
to use for SNMP queries
- `inlet``core``exporter-classifiers` to define rules to attach
attributes to your exporters
- `inlet``core``interface-classifiers` to define rules to attach

View File

@@ -255,10 +255,10 @@ continuously the exporters. The following keys are accepted:
about to expire or need an update
- `cache-persist-file` tells where to store cached data on shutdown and
read them back on startup
- `default-community` tells which community to use when polling exporters
- `communities` is a map from a subnets to the community to use for
exporters in the provided subnet. Use `::/0` to set the default
value. Alternatively, it also accepts a string to use for all
exporters.
exporters in the provided subnet, overriding the default value
above.
- `poller-retries` is the number of retries on unsuccessful SNMP requests.
- `poller-timeout` tells how much time should the poller wait for an answer.
- `workers` tell how many workers to spawn to handle SNMP polling.

View File

@@ -23,7 +23,7 @@ details.
- 🩹 *orchestrator*: fix `SrcCountry`/`DstCountry` columns in aggregated tables [PR #61][]
- 🌱 *inlet*: `inlet.geoip.country-database` has been renamed to `inlet.geoip.geo-database`
- 🌱 *inlet*: add counters for GeoIP database hit/miss
- 🌱 *inlet*: `inlet.snmp.communities` accepts subnets as keys, `inlet.snmp.default-community` is now deprecated
- 🌱 *inlet*: `inlet.snmp.communities` accepts subnets as keys
- 🌱 *docker-compose*: disable healthcheck for the conntrack-fixer container
[PR #61]: https://github.com/vincentbernat/akvorado/pull/61

View File

@@ -4,12 +4,9 @@
package snmp
import (
"reflect"
"time"
"akvorado/common/helpers"
"github.com/mitchellh/mapstructure"
)
// Configuration describes the configuration for the SNMP client
@@ -22,6 +19,8 @@ type Configuration struct {
CacheCheckInterval time.Duration `validate:"ltefield=CacheRefresh"`
// CachePersist defines a file to store cache and survive restarts
CachePersistFile string
// DefaultCommunity is the default SNMP community to use
DefaultCommunity string `validate:"required"`
// Communities is a mapping from exporter IPs to communities
Communities *helpers.SubnetMap[string]
// PollerRetries tell how many time a poller should retry before giving up
@@ -36,18 +35,12 @@ type Configuration struct {
// DefaultConfiguration represents the default configuration for the SNMP client.
func DefaultConfiguration() Configuration {
communities, err := helpers.NewSubnetMap(map[string]string{
"::/0": "public",
})
if err != nil {
panic(err)
}
return Configuration{
CacheDuration: 30 * time.Minute,
CacheRefresh: time.Hour,
CacheCheckInterval: 2 * time.Minute,
CachePersistFile: "",
Communities: communities,
DefaultCommunity: "public",
PollerRetries: 1,
PollerTimeout: time.Second,
PollerCoalesce: 10,
@@ -55,42 +48,6 @@ func DefaultConfiguration() Configuration {
}
}
// ConfigurationUnmarshallerHook normalize SNMP configuration:
// - append default-community to communities (as ::/0)
func ConfigurationUnmarshallerHook() mapstructure.DecodeHookFunc {
return func(from, to reflect.Value) (interface{}, error) {
if from.Kind() != reflect.Map || from.IsNil() || from.Type().Key().Kind() != reflect.String || to.Type() != reflect.TypeOf(Configuration{}) {
return from.Interface(), nil
}
// default-community → communities
var defaultKey, mapKey *reflect.Value
fromMap := from.MapKeys()
for i, k := range fromMap {
if helpers.MapStructureMatchName(k.String(), "DefaultCommunity") {
defaultKey = &fromMap[i]
} else if helpers.MapStructureMatchName(k.String(), "Communities") {
mapKey = &fromMap[i]
}
}
if defaultKey != nil {
if mapKey == nil {
from.SetMapIndex(reflect.ValueOf("communities"), from.MapIndex(*defaultKey))
} else {
communities := from.MapIndex(*mapKey)
if communities.Kind() == reflect.Interface {
communities = communities.Elem()
}
communities.SetMapIndex(reflect.ValueOf("::/0"), from.MapIndex(*defaultKey))
}
from.SetMapIndex(*defaultKey, reflect.Value{})
}
return from.Interface(), nil
}
}
func init() {
helpers.AddMapstructureUnmarshallerHook(ConfigurationUnmarshallerHook())
helpers.AddMapstructureUnmarshallerHook(helpers.SubnetMapUnmarshallerHook[string]())
}

View File

@@ -5,12 +5,8 @@ package snmp
import (
"testing"
"time"
"akvorado/common/helpers"
"github.com/gin-gonic/gin"
"github.com/mitchellh/mapstructure"
)
func TestDefaultConfiguration(t *testing.T) {
@@ -18,84 +14,3 @@ func TestDefaultConfiguration(t *testing.T) {
t.Fatalf("validate.Struct() error:\n%+v", err)
}
}
func TestConfigurationUnmarshallerHook(t *testing.T) {
cases := []struct {
Description string
Input gin.H
Output Configuration
}{
{
Description: "nil",
Input: nil,
}, {
Description: "empty",
Input: gin.H{},
}, {
Description: "no communities, no default community",
Input: gin.H{
"cache-refresh": "10s",
"poller-retries": 10,
},
Output: Configuration{
CacheRefresh: 10 * time.Second,
PollerRetries: 10,
},
}, {
Description: "communities, no default community",
Input: gin.H{
"communities": gin.H{
"203.0.113.0/25": "public",
"203.0.113.128/25": "private",
},
},
Output: Configuration{
Communities: helpers.MustNewSubnetMap(map[string]string{
"::ffff:203.0.113.0/121": "public",
"::ffff:203.0.113.128/121": "private",
}),
},
}, {
Description: "no communities, default community",
Input: gin.H{
"default-community": "private",
},
Output: Configuration{
Communities: helpers.MustNewSubnetMap(map[string]string{
"::/0": "private",
}),
},
}, {
Description: "communities, default community",
Input: gin.H{
"default-community": "private",
"communities": gin.H{
"203.0.113.0/25": "public",
"203.0.113.128/25": "private",
},
},
Output: Configuration{
Communities: helpers.MustNewSubnetMap(map[string]string{
"::/0": "private",
"::ffff:203.0.113.0/121": "public",
"::ffff:203.0.113.128/121": "private",
}),
},
},
}
for _, tc := range cases {
t.Run(tc.Description, func(t *testing.T) {
var got Configuration
decoder, err := mapstructure.NewDecoder(helpers.GetMapStructureDecoderConfig(&got))
if err != nil {
t.Fatalf("NewDecoder() error:\n%+v", err)
}
err = decoder.Decode(tc.Input)
if err != nil {
t.Fatalf("Decode() error:\n%+v", err)
} else if diff := helpers.Diff(got, tc.Output); diff != "" {
t.Fatalf("Decode() (-got, +want):\n%s", diff)
}
})
}
}

View File

@@ -285,7 +285,7 @@ func (c *Component) dispatchIncomingRequest(request lookupRequest) {
func (c *Component) pollerIncomingRequest(request lookupRequest) {
community, ok := c.config.Communities.Lookup(net.ParseIP(request.ExporterIP))
if !ok {
community = "public"
community = c.config.DefaultCommunity
}
// Avoid querying too much exporters with errors

View File

@@ -41,8 +41,8 @@ func TestLookup(t *testing.T) {
func TestSNMPCommunities(t *testing.T) {
r := reporter.NewMock(t)
configuration := DefaultConfiguration()
configuration.Communities, _ = helpers.NewSubnetMap(map[string]string{
"::/0": "notpublic",
configuration.DefaultCommunity = "notpublic"
configuration.Communities = helpers.MustNewSubnetMap(map[string]string{
"::ffff:127.0.0.1/128": "public",
"::ffff:127.0.0.2/128": "private",
})