fix: Correct multiple labels filtering for get commadn (#59)

## Motivation

Our backend only interprets the first query parameter value it receives.
Since we've been setting `url.Values` labels query parameter key to a
list of parsed values, this caused our backend to only interpret the
first value.

With example Projects:

```yaml
- apiVersion: n9/v1alpha
  kind: Project
  metadata:
    displayName: azure-agent
    labels:
      env:
      - dev
      - dev-demo-1
      team:
      - bees
    name: azure-agent
  spec:
    description: azure-agent
- apiVersion: n9/v1alpha
  kind: Project
  metadata:
    labels:
      team:
      - bees
    name: project-two
  spec:
    description: ""
```

When running:

```sh
sloctl get project -l env=dev,team=bees
sloctl get project -l team=bees,env=dev
```

We got:
- for the first command, both Projects
- for the second one, only one Project, `azure-agent`

**What should've happened?**

Both commands should only return `azure-agent`, as this should've been
interpreted as AND condition between labels. As a side effect, OR
conditions have been broken too (same key, multiple labels), so these
should work now too.

## Testing

- Extended e2e tests coverage.

## Release Notes

Fixed labels filtering for `sloctl get` command which was causing
incorrect results to be returned by our backend.
Issue affects every sloctl version starting from and including v0.0.95.

internal/get.go

@@ -358,9 +358,9 @@ func (g *GetCmd) getObjects(ctx context.Context, args []string, kind manifest.Ki
 		return nil, errors.New("'sloctl get alerts' does not support Project filtering," +
 			" explicitly pass '-A' flag to fetch all Alerts.")
 	}
-	query := url.Values{
-		objectsV1.QueryKeyName:   args,
-		objectsV1.QueryKeyLabels: parseFilterLabel(g.labels),
+	query := url.Values{objectsV1.QueryKeyName: args}
+	if len(g.labels) > 0 {
+		query.Set(objectsV1.QueryKeyLabels, parseFilterLabel(g.labels))
 	}
 	header := http.Header{sdk.HeaderProject: []string{g.client.Config.Project}}
 	objects, err := g.client.Objects().V1().Get(ctx, kind, header, query)
@@ -379,7 +379,7 @@ func (g *GetCmd) getObjects(ctx context.Context, args []string, kind manifest.Ki
 	return objects, nil
 }
 
-func parseFilterLabel(filterLabels []string) []string {
+func parseFilterLabel(filterLabels []string) string {
 	labels := make(v1alpha.Labels)
 	for _, filterLabel := range filterLabels {
 		filteredLabels := strings.Split(filterLabel, ",")
@@ -404,7 +404,7 @@ func parseFilterLabel(filterLabels []string) []string {
 			strLabels = append(strLabels, key)
 		}
 	}
-	return strLabels
+	return strings.Join(strLabels, ",")
 }
 
 func (g *GetCmd) printObjects(objects interface{}) error {

test/get.bats

@@ -84,19 +84,65 @@ setup() {
 }
 
 @test "agent with keys" {
-  for flag in -k --with-keys; do
-	  run_sloctl get agent -p "death-star" "$flag"
-	  assert_success
-    # Assert length of client_id and regex of client_secret, as the latter may vary.
-	  client_id="$(yq -r .[].metadata.client_id <<<"$output")"
-	  client_secret="$(yq -r .[].metadata.client_secret <<<"$output")"
-	  assert_equal "${#client_id}" 20
-	  assert_regex "${#client_secret}" "[a-zA-Z0-9_-]+"
-    # Finally make sure the whole Agent definition is being presented.
+	for flag in -k --with-keys; do
+		run_sloctl get agent -p "death-star" "$flag"
+		assert_success
+		# Assert length of client_id and regex of client_secret, as the latter may vary.
+		client_id="$(yq -r .[].metadata.client_id <<<"$output")"
+		client_secret="$(yq -r .[].metadata.client_secret <<<"$output")"
+		assert_equal "${#client_id}" 20
+		assert_regex "${#client_secret}" "[a-zA-Z0-9_-]+"
+		# Finally make sure the whole Agent definition is being presented.
 		verify_get_success "$output" "$(read_files "${TEST_INPUTS}/agent.yaml")"
-  done
+	done
+}
+
+@test "projects, multiple names" {
+	run_sloctl get project death-star hoth-base
+	verify_get_success "$output" "$(read_files "${TEST_INPUTS}/projects.yaml")"
 }
 
+@test "projects, labels filtering, OR conditions" {
+	want=$(read_files "${TEST_INPUTS}/projects.yaml")
+	for label in \
+		"-l purpose=defensive" \
+		"-l purpose=offensive,purpose=defensive" \
+		"-l purpose=defensive,purpose=offensive" \
+		"-l purpose=defensive -l purpose=offensive" \
+		"-l purpose=offensive -l purpose=defensive"; do
+		run_sloctl get project "$label"
+		verify_get_success "$output" "$want"
+	done
+}
+
+@test "projects, labels filtering, AND conditions" {
+	want=$(read_files "${TEST_INPUTS}/projects.yaml" | yq -r '.[] |= select(.metadata.name == "death-star")')
+	for label in \
+		"-l purpose=offensive" \
+		"-l purpose=defensive,team=vader" \
+		"-l purpose=offensive,team=vader" \
+		"-l purpose=offensive,purpose=defensive,team=sidious" \
+		"-l team=sidious,purpose=offensive,purpose=defensive" \
+		"-l team=sidious,purpose=defensive,purpose=offensive" \
+		"-l purpose=offensive -l purpose=defensive,team=sidious" \
+		"-l purpose=offensive -l team=sidious,purpose=defensive" \
+		"-l team=sidious -l purpose=offensive -l purpose=defensive" \
+		"-l purpose=defensive -l purpose=offensive -l team=sidious" \
+		"-l purpose=offensive -l purpose=defensive -l team=sidious"; do
+		run_sloctl get project "$label"
+		verify_get_success "$output" "$want"
+	done
+}
+
+@test "projects, labels filtering with name" {
+	run_sloctl get project -l purpose=defensive hoth-base
+	want=$(read_files "${TEST_INPUTS}/projects.yaml" | yq -r '.[] |= select(.metadata.name == "hoth-base")')
+	verify_get_success "$output" "$want"
+
+	run_sloctl get project -l purpose=offensive hoth-base
+	assert_success
+	assert_output "No resources found."
+}
 
 test_get() {
 	local \
@@ -145,11 +191,11 @@ test_get() {
 
 	for alias in "${aliases[@]}"; do
 		if [[ "$kind" == "Project" ]] || [[ "$kind" == "UserGroup" ]]; then
-		  run_sloctl get "$alias" "fake-name-123-321"
-		  assert_success
-		  assert_output "No resources found."
+			run_sloctl get "$alias" "fake-name-123-321"
+			assert_success
+			assert_output "No resources found."
 
-		  continue
+			continue
 		fi
 
 		run_sloctl get "$alias" "fake-name-123-321"
@@ -170,7 +216,7 @@ verify_get_success() {
 	# we need to hack our way around it.
 	refute_output --partial "Available Commands:"
 	# We can't retrieve the same object we applied so we need to compare the minimum.
-	filter='[.[] | {"name": .metadata.name, "project": .metadata.project}] | sort_by(.name, .project)'
+	filter='[.[] | {"name": .metadata.name, "project": .metadata.project, "labels": .metadata.labels}] | sort_by(.name, .project)'
 	assert_equal \
 		"$(yq --sort-keys -y -r "$filter" <<<"$have")" \
 		"$(yq --sort-keys -y -r "$filter" <<<"$want")"

test/inputs/get/projects.yaml

@@ -2,12 +2,24 @@
   kind: Project
   metadata:
     name: death-star
+    labels:
+      purpose:
+      - defensive
+      - offensive
+      team:
+      - vader
+      - sidious
   spec:
     description: Dummy Project for 'sloctl get' e2e tests
 - apiVersion: n9/v1alpha
   kind: Project
   metadata:
     displayName: Hoth Base
     name: hoth-base
+    labels:
+      purpose:
+      - defensive
+      team:
+      - solo
   spec:
     description: Dummy Project for 'sloctl get' e2e tests

test/test_helper/load.bash

@@ -12,7 +12,7 @@
 # The output of sloctl is sanitized, the trailing whitespaces,
 # if present, are removed for easier output validation.
 run_sloctl() {
-  run bash -c "set -o pipefail && sloctl $* | sed 's/ *$//'"
+	run bash -c "set -o pipefail && sloctl $* | sed 's/ *$//'"
 }
 
 # read_files
@@ -31,7 +31,7 @@ run_sloctl() {
 # documents style.
 # yq works with json as it is only a preprocessor for jq.
 read_files() {
-  yq -sY '[ .[] | if type == "array" then .[] else . end]' "$@"
+	yq -sY '[ .[] | if type == "array" then .[] else . end]' "$@"
 }
 
 # assert_applied
@@ -44,7 +44,7 @@ read_files() {
 # Options:
 #   <expected>    The expected YAML string.
 assert_applied() {
-  _assert_objects_existence "apply" "$1"
+	_assert_objects_existence "apply" "$1"
 }
 
 # assert_deleted
@@ -57,7 +57,7 @@ assert_applied() {
 # Options:
 #   <expected>    The expected YAML string.
 assert_deleted() {
-  _assert_objects_existence "delete" "$1"
+	_assert_objects_existence "delete" "$1"
 }
 
 # _assert_objects_existence
@@ -80,44 +80,44 @@ assert_deleted() {
 # - apply: assert that the output contains the expected object.
 # - delete: assert that the output contains 'No resources found'.
 _assert_objects_existence() {
-  load_lib "bats-support"
-
-  assert [ -n "$2" ]
-  assert [ "$(yq -r 'type' <<<"$2")" = "array" ]
-
-  yq -c .[] <<<"$2" | while read -r object; do
-    name=$(yq -r .metadata.name <<<"$object")
-    kind=$(yq -r .kind <<<"$object")
-    args=("get" "${kind,,}" "$name") # Converts kind to lowercase.
-    if [[ "$kind" != "Project" ]] && [[ "$kind" != "RoleBinding" ]]; then
-      project=$(yq -r .metadata.project <<<"$object")
-      args+=(-p "$project")
-    fi
-
-    case "$1" in
-      apply)
-        run_sloctl "${args[*]}"
-        # shellcheck disable=2154
-        have=$(yq --sort-keys -y '[.[] | del(.status)]' <<<"$output")
-        want=$(yq --sort-keys -y '[
+	load_lib "bats-support"
+
+	assert [ -n "$2" ]
+	assert [ "$(yq -r 'type' <<<"$2")" = "array" ]
+
+	yq -c .[] <<<"$2" | while read -r object; do
+		name=$(yq -r .metadata.name <<<"$object")
+		kind=$(yq -r .kind <<<"$object")
+		args=("get" "${kind,,}" "$name") # Converts kind to lowercase.
+		if [[ "$kind" != "Project" ]] && [[ "$kind" != "RoleBinding" ]]; then
+			project=$(yq -r .metadata.project <<<"$object")
+			args+=(-p "$project")
+		fi
+
+		case "$1" in
+		apply)
+			run_sloctl "${args[*]}"
+			# shellcheck disable=2154
+			have=$(yq --sort-keys -y '[.[] | del(.status)]' <<<"$output")
+			want=$(yq --sort-keys -y '[
           .[] | select(.kind == "'"$kind"'") |
           select(.metadata.name == "'"$name"'") |
           if .metadata.project then
             select(.metadata.project == "'"$project"'")
           else
             .
           end]' <<<"$2")
-        assert_equal "$have" "$want"
-        ;;
-      delete)
-        run_sloctl "${args[*]}"
-        assert_output --partial "No resources found"
-        ;;
-      *)
-        fail "Unknown verb '$1'"
-        ;;
-    esac
-  done
+			assert_equal "$have" "$want"
+			;;
+		delete)
+			run_sloctl "${args[*]}"
+			assert_output --partial "No resources found"
+			;;
+		*)
+			fail "Unknown verb '$1'"
+			;;
+		esac
+	done
 }
 
 # generate_inputs
@@ -137,44 +137,44 @@ _assert_objects_existence() {
 # them in parallel or a cleanup after the test fails for whatever reason.
 # It works for both YAML and JSON files.
 generate_inputs() {
-  load_lib "bats-support"
+	load_lib "bats-support"
 
-  directory="$1"
-  test_filename=$(basename "$BATS_TEST_FILENAME" .bats)
-  TEST_INPUTS="$directory/$test_filename"
-  mkdir "$TEST_INPUTS"
+	directory="$1"
+	test_filename=$(basename "$BATS_TEST_FILENAME" .bats)
+	TEST_INPUTS="$directory/$test_filename"
+	mkdir "$TEST_INPUTS"
 
-  test_hash="${BATS_TEST_NUMBER}-$(date +%s)-$SLOCTL_GIT_REVISION"
-  TEST_PROJECT="e2e-$test_hash"
+	test_hash="${BATS_TEST_NUMBER}-$(date +%s)-$SLOCTL_GIT_REVISION"
+	TEST_PROJECT="e2e-$test_hash"
 
-  files=$(find "$TEST_SUITE_INPUTS/$test_filename" -type f \( -iname \*.json -o -iname \*.yaml -o -iname \*.yml \))
-  for file in $files; do
-    pipeline='
+	files=$(find "$TEST_SUITE_INPUTS/$test_filename" -type f \( -iname \*.json -o -iname \*.yaml -o -iname \*.yml \))
+	for file in $files; do
+		pipeline='
       if .kind == "Project" then
-        .metadata.labels = {"origin": ["sloctl-e2e-tests"]}
+        .metadata.labels.origin = ["sloctl-e2e-tests"]
       else
         .
       end'
-    filter='
+		filter='
       if type == "array" then
         [.[] | '"$pipeline"' ]
       else
         '"$pipeline"'
       end'
-    new_file="${file/$TEST_SUITE_INPUTS/$directory}"
-    mkdir -p "$(dirname "$new_file")"
-    sed_replace="s/<PROJECT>/$TEST_PROJECT/g"
-    if [[ $file =~ .*.ya?ml ]]; then
-      yq -Y "$filter" "$file" | sed "$sed_replace" >"$new_file"
-    elif [[ $file == *.json ]]; then
-      jq "$filter" "$file" | sed "$sed_replace" >"$new_file"
-    else
-      fail "test input file: ${file} must be either YAML or JSON"
-    fi
-  done
-
-  export TEST_INPUTS
-  export TEST_PROJECT
+		new_file="${file/$TEST_SUITE_INPUTS/$directory}"
+		mkdir -p "$(dirname "$new_file")"
+		sed_replace="s/<PROJECT>/$TEST_PROJECT/g"
+		if [[ $file =~ .*.ya?ml ]]; then
+			yq -Y "$filter" "$file" | sed "$sed_replace" >"$new_file"
+		elif [[ $file == *.json ]]; then
+			jq "$filter" "$file" | sed "$sed_replace" >"$new_file"
+		else
+			fail "test input file: ${file} must be either YAML or JSON"
+		fi
+	done
+
+	export TEST_INPUTS
+	export TEST_PROJECT
 }
 
 # select_object
@@ -192,7 +192,7 @@ generate_inputs() {
 # extract an object by its former name a regex match with jq 'test'
 # function has to be performed.
 select_object() {
-  yq '[if type == "array" then .[] else . end |
+	yq '[if type == "array" then .[] else . end |
     select(.metadata.name | test("^'"$1"'"))]' "$1" "$2"
 }
 
@@ -208,16 +208,16 @@ select_object() {
 #
 # If 'yq' is provided as one of the dependencies, ensure it is coming from https://github.com/kislyuk/yq.
 ensure_installed() {
-  load_lib "bats-support"
-
-  for dep in "$@"; do
-    if ! command -v "$dep" >/dev/null 2>&1; then
-      fail "ERROR: $dep is not installed!"
-    fi
-    if [ "$dep" = "yq" ] && [ "$(yq --help | grep "kislyuk/yq")" -eq 1 ]; then
-      fail "ERROR: yq is not installed from https://github.com/kislyuk/yq!"
-    fi
-  done
+	load_lib "bats-support"
+
+	for dep in "$@"; do
+		if ! command -v "$dep" >/dev/null 2>&1; then
+			fail "ERROR: $dep is not installed!"
+		fi
+		if [ "$dep" = "yq" ] && [ "$(yq --help | grep "kislyuk/yq")" -eq 1 ]; then
+			fail "ERROR: yq is not installed from https://github.com/kislyuk/yq!"
+		fi
+	done
 }
 
 # load_lib
@@ -230,6 +230,6 @@ ensure_installed() {
 # Options:
 #   <name>    Name of the library to load.
 load_lib() {
-  local name="$1"
-  load "/usr/lib/bats/${name}/load.bash"
+	local name="$1"
+	load "/usr/lib/bats/${name}/load.bash"
 }