Skip to content

Commit

Permalink
Use labels.SelectorFromValidatedSet instead.
Browse files Browse the repository at this point in the history
Make filters.Matches return true on empty map.
Un-export byType.
Rename Get/GetElastic => GetByLabel/GetByName.
DRY up the Get* methods.
  • Loading branch information
naemono committed Nov 14, 2022
1 parent 173ce66 commit d25bbe3
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 86 deletions.
2 changes: 1 addition & 1 deletion internal/agentdiag.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func runAgentDiagnostics(k *Kubectl, ns string, zipFile *archive.ZipFile, verbos
return nil
}

if !filters.Empty() && !filters.Matches(labels) {
if !filters.Matches(labels) {
return nil
}

Expand Down
20 changes: 10 additions & 10 deletions internal/diag.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ func Run(params Params) error {
return kubectl.Version(writer)
},
"nodes.json": func(writer io.Writer) error {
return kubectl.Get("nodes", "", filters.Filters{}, writer)
return kubectl.GetByLabel("nodes", "", filters.Filters{}, writer)
},
"podsecuritypolicies.json": func(writer io.Writer) error {
return kubectl.Get("podsecuritypolicies", "", filters.Filters{}, writer)
return kubectl.GetByLabel("podsecuritypolicies", "", filters.Filters{}, writer)
},
"storageclasses.json": func(writer io.Writer) error {
return kubectl.Get("storageclasses", "", filters.Filters{}, writer)
return kubectl.GetByLabel("storageclasses", "", filters.Filters{}, writer)
},
"clusterroles.txt": func(writer io.Writer) error {
return kubectl.Describe("clusterroles", "elastic", "", writer)
Expand All @@ -113,7 +113,7 @@ func Run(params Params) error {

operatorVersions = append(operatorVersions, detectECKVersion(clientSet, ns, params.ECKVersion))

zipFile.Add(getResources(kubectl.Get, ns, filters.Filters{}, []string{
zipFile.Add(getResources(kubectl.GetByLabel, ns, filters.Filters{}, []string{
"statefulsets",
"pods",
"services",
Expand Down Expand Up @@ -148,7 +148,7 @@ LOOP:
default:
}
logger.Printf("Extracting Kubernetes diagnostics from %s\n", ns)
zipFile.Add(getResources(kubectl.Get, ns, params.Filters, []string{
zipFile.Add(getResources(kubectl.GetByLabel, ns, params.Filters, []string{
"statefulsets",
"replicasets",
"deployments",
Expand All @@ -161,36 +161,36 @@ LOOP:
"controllerrevisions",
}))

zipFile.Add(getResources(kubectl.GetElastic, ns, params.Filters, []string{
zipFile.Add(getResources(kubectl.GetByName, ns, params.Filters, []string{
"kibana",
"elasticsearch",
"apmserver",
}))

// Filters is intentionally empty here, as Elastic labels
// are not applied to these resources.
zipFile.Add(getResources(kubectl.Get, ns, filters.Filters{}, []string{
zipFile.Add(getResources(kubectl.GetByLabel, ns, filters.Filters{}, []string{
"persistentvolumes",
"events",
"networkpolicies",
"serviceaccount",
}))

if maxOperatorVersion.AtLeast(version.MustParseSemantic("1.2.0")) {
zipFile.Add(getResources(kubectl.GetElastic, ns, params.Filters, []string{
zipFile.Add(getResources(kubectl.GetByName, ns, params.Filters, []string{
"enterprisesearch",
"beat",
}))
}

if maxOperatorVersion.AtLeast(version.MustParseSemantic("1.4.0")) {
zipFile.Add(getResources(kubectl.GetElastic, ns, params.Filters, []string{
zipFile.Add(getResources(kubectl.GetByName, ns, params.Filters, []string{
"agent",
}))
}

if maxOperatorVersion.AtLeast(version.MustParseSemantic("1.6.0")) {
zipFile.Add(getResources(kubectl.GetElastic, ns, params.Filters, []string{
zipFile.Add(getResources(kubectl.GetByName, ns, params.Filters, []string{
"elasticmapsserver",
}))
}
Expand Down
22 changes: 13 additions & 9 deletions internal/filters/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,22 @@ var (

// Filters contains a Filter map for each Elastic type given in the filter "source".
type Filters struct {
ByType map[string][]Filter
byType map[string][]Filter
}

// Empty simply returns if there are no defined filters.
// Empty returns if there are no defined filters.
func (f Filters) Empty() bool {
return len(f.ByType) == 0
return len(f.byType) == 0
}

// Matches will determine if the given labels matches any of the
// Filter's label selectors.
func (f Filters) Matches(lbls map[string]string) bool {
for _, fs := range f.ByType {
// empty set of filters always matches.
if f.Empty() {
return true
}
for _, fs := range f.byType {
for _, filter := range fs {
if filter.Selector.Matches(labels.Set(lbls)) {
return true
Expand All @@ -48,7 +52,7 @@ func (f Filters) Contains(name, typ string) bool {
ok bool
typeFilters []Filter
)
if typeFilters, ok = f.ByType[typ]; !ok {
if typeFilters, ok = f.byType[typ]; !ok {
return false
}
for _, filter := range typeFilters {
Expand Down Expand Up @@ -83,7 +87,7 @@ func New(source []string) (Filters, error) {
// returning the set of Filters, and any errors encountered.
func parse(source []string) (Filters, error) {
filters := Filters{
ByType: map[string][]Filter{},
byType: map[string][]Filter{},
}
if len(source) == 0 {
return filters, nil
Expand All @@ -98,14 +102,14 @@ func parse(source []string) (Filters, error) {
if err := validateType(typ); err != nil {
return filters, err
}
if _, ok := filters.ByType[typ]; ok {
if _, ok := filters.byType[typ]; ok {
return filters, fmt.Errorf("invalid filter: %s: multiple filters for the same type (%s) are not supported", fltr, typ)
}
selector, err := processSelector(typ, name)
if err != nil {
return filters, fmt.Errorf("while processing label selector: %w", err)
}
filters.ByType[typ] = append(filters.ByType[typ], Filter{
filters.byType[typ] = append(filters.byType[typ], Filter{
Name: name,
Type: typ,
Selector: selector,
Expand Down Expand Up @@ -140,7 +144,7 @@ func processSelector(typ, name string) (labels.Selector, error) {
"common.k8s.elastic.co/type": typ,
fmt.Sprintf("%s.k8s.elastic.co/%s", typ, nameAttr): name,
}
s := labels.SelectorFromSet(set)
s := labels.SelectorFromValidatedSet(set)
for sk, v := range set {
req, err := labels.NewRequirement(sk, selection.Equals, []string{v})
if err != nil {
Expand Down
28 changes: 14 additions & 14 deletions internal/filters/filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ func TestNew(t *testing.T) {
{
name: "empty/no filter is valid",
filters: []string{},
want: Filters{ByType: map[string][]Filter{}},
want: Filters{byType: map[string][]Filter{}},
wantErr: false,
},
{
name: "valid name and agent type is valid",
filters: []string{"agent=myagent"},
want: Filters{ByType: map[string][]Filter{
want: Filters{byType: map[string][]Filter{
"agent": {{
Type: "agent",
Name: "myagent",
Expand All @@ -44,7 +44,7 @@ func TestNew(t *testing.T) {
{
name: "valid name and apm type is valid",
filters: []string{"apm=myapm"},
want: Filters{ByType: map[string][]Filter{
want: Filters{byType: map[string][]Filter{
"apm": {{
Type: "apm",
Name: "myapm",
Expand All @@ -58,7 +58,7 @@ func TestNew(t *testing.T) {
{
name: "valid name and beat type is valid",
filters: []string{"beat=mybeat"},
want: Filters{ByType: map[string][]Filter{
want: Filters{byType: map[string][]Filter{
"beat": {{
Type: "beat",
Name: "mybeat",
Expand All @@ -72,7 +72,7 @@ func TestNew(t *testing.T) {
{
name: "valid name and elasticsearch type is valid",
filters: []string{"elasticsearch=mycluster"},
want: Filters{ByType: map[string][]Filter{
want: Filters{byType: map[string][]Filter{
"elasticsearch": {{
Type: "elasticsearch",
Name: "mycluster",
Expand All @@ -86,7 +86,7 @@ func TestNew(t *testing.T) {
{
name: "valid name and enterprisesearch type is valid",
filters: []string{"enterprisesearch=mycluster"},
want: Filters{ByType: map[string][]Filter{
want: Filters{byType: map[string][]Filter{
"enterprisesearch": {{
Type: "enterprisesearch",
Name: "mycluster",
Expand All @@ -100,7 +100,7 @@ func TestNew(t *testing.T) {
{
name: "valid name and kibana type is valid",
filters: []string{"kibana=mykb"},
want: Filters{ByType: map[string][]Filter{
want: Filters{byType: map[string][]Filter{
"kibana": {{
Type: "kibana",
Name: "mykb",
Expand All @@ -114,7 +114,7 @@ func TestNew(t *testing.T) {
{
name: "valid name and maps type is valid",
filters: []string{"maps=mymaps"},
want: Filters{ByType: map[string][]Filter{
want: Filters{byType: map[string][]Filter{
"maps": {{
Type: "maps",
Name: "mymaps",
Expand All @@ -128,7 +128,7 @@ func TestNew(t *testing.T) {
{
name: "multiple valid filters return correctly",
filters: []string{"elasticsearch=mycluster", "kibana=my-kb", "agent=my-agent"},
want: Filters{ByType: map[string][]Filter{
want: Filters{byType: map[string][]Filter{
"agent": {{
Type: "agent",
Name: "my-agent",
Expand Down Expand Up @@ -156,7 +156,7 @@ func TestNew(t *testing.T) {
{
name: "invalid type is invalid",
filters: []string{"type=invalid"},
want: Filters{ByType: map[string][]Filter{}},
want: Filters{byType: map[string][]Filter{}},
wantErr: true,
},
}
Expand Down Expand Up @@ -194,7 +194,7 @@ func TestFilters_Matches(t *testing.T) {
selector.Add(*req)
}
defaultFilters := Filters{
ByType: map[string][]Filter{
byType: map[string][]Filter{
"elasticsearch": {{
Type: "elasticsearch",
Name: "my-cluster",
Expand Down Expand Up @@ -231,7 +231,7 @@ func TestFilters_Matches(t *testing.T) {
"elasticsearch.k8s.elastic.co/version": "8.2.3",
"statefulset.kubernetes.io/pod-name": "my-cluster-es-default-0",
},
filterMap: defaultFilters.ByType,
filterMap: defaultFilters.byType,
want: true,
},
{
Expand All @@ -242,14 +242,14 @@ func TestFilters_Matches(t *testing.T) {
"common.k8s.elastic.co/type": "agent",
"pod-template-hash": "7cbfdc4d78",
},
filterMap: defaultFilters.ByType,
filterMap: defaultFilters.byType,
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f := Filters{
ByType: tt.filterMap,
byType: tt.filterMap,
}
if got := f.Matches(tt.labels); got != tt.want {
t.Errorf("Filters.Matches() = %v, want %v", got, tt.want)
Expand Down
Loading

0 comments on commit d25bbe3

Please sign in to comment.