Add select var type

[jillguyonnet]

What does this PR do?

This PR adds a new select var type for integrations.

As integrations need a way to provide a list of options for the select, it also adds an options property which is only relevant to select types.

The options property is conditionally added, such as it is required when type is select and required not to be there for other types. The conditional rule to enforce this was inspired from this post.

I also added a requirement for the options array to contain at least one element.

Why is it important?

As detailed in #453, the Elasticsearch integration has an setting with only two valid options. Currently, this option is set in the UI via a text field with a description, which is error prone. Adding a select (dropdown) var type will enhance this UX.

Checklist

- [ ] I have added test packages to test/packages that prove my change is effective.

• I have added an entry in spec/changelog.yml.

Related issues

• Relates [Change Proposal] Add support for "select/option" type for Integration parameters #453

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2023-03-21T14:43:01.869+0000

• Duration: 9 min 18 sec

Test stats 🧪

Test	Results
Failed	0
Passed	716
Skipped	0
Total	716

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

[elasticmachine]

🌐 Coverage report

Name	Metrics % (covered/total)	Diff
Packages	100.0% (7/7)	💚
Files	69.231% (18/26)	👍
Classes	77.143% (27/35)	👍
Methods	56.757% (63/111)	👍 0.793
Lines	42.727% (564/1320)	👍 0.622
Conditionals	100.0% (0/0)	💚

[mrodm]

That new type select and the related options object look a good approach 👍

Please, update the test package good_v2 to include a new field using this new select type with some options.

And add also some new test "bad" package, for instance, with a package where the options is defined but with another type. It should be added here a new test case

package-spec/code/go/pkg/validator/validator_test.go

Line 21 in 6056037

func TestValidateFile(t *testing.T) {

[jillguyonnet]

@mrodm I pushed a change with conditional subschema thanks to your suggestion and help 🙂

[jsoriano]

Oh nice, maybe we can use if/then in more cases where we are validating with go now.

[jsoriano]

Nit. I tend to prefer to avoid elses and have each exception on its own if:

Suggested change

	if strings.Contains(e.Error(), msg.original) {
	processedErrs = append(processedErrs, errors.New(strings.Replace(e.Error(), msg.original, msg.new, 1)))
	} else if !substringInSlice(e.Error(), redundant) {
	processedErrs = append(processedErrs, e)
	}
	if strings.Contains(e.Error(), msg.original) {
	processedErrs = append(processedErrs, errors.New(strings.Replace(e.Error(), msg.original, msg.new, 1)))
	continue
	}
	if substringInSlice(e.Error(), redundant) {
	continue
	}
	processedErrs = append(processedErrs, e)

Or maybe use a switch.

Suggested change

	if strings.Contains(e.Error(), msg.original) {
	processedErrs = append(processedErrs, errors.New(strings.Replace(e.Error(), msg.original, msg.new, 1)))
	} else if !substringInSlice(e.Error(), redundant) {
	processedErrs = append(processedErrs, e)
	}
	switch {
	case strings.Contains(e.Error(), msg.original):
	processedErrs = append(processedErrs, errors.New(strings.Replace(e.Error(), msg.original, msg.new, 1)))
	case substringInSlice(e.Error(), redundant):
	default:
	processedErrs = append(processedErrs, e)
	}

[jillguyonnet]

Thanks! I think your first suggestion reads better, I've pushed the change.

[jsoriano]

👍

[mrodm]

LGTM!

[mrodm]

Perfect, this is more helpful for the developers 👍

[jillguyonnet]

Thank you for the review @jsoriano and @mrodm 👍 I've rebased the branch to update spec/changelog.yml.