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

chore(cli): Adding support for Variable Sets #3018

Merged
merged 6 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions cli/cmd/middleware.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package cmd

import (
"errors"
"fmt"
"os"

"github.com/kubeshop/tracetest/cli/pkg/resourcemanager"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -84,8 +82,8 @@ func (p *resourceParameters) Validate(cmd *cobra.Command, args []string) []error

p.ResourceName = args[0]

_, err := resources.Get(p.ResourceName)
if errors.Is(err, resourcemanager.ErrResourceNotFound) {
exists := resources.Exists(p.ResourceName)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to use a different function because Get now writes the warning to the console if deprecated

if !exists {
suggestion := resources.Suggest(p.ResourceName)
if suggestion != "" {
return []error{
Expand Down
11 changes: 11 additions & 0 deletions cli/cmd/resource_run_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ func init() {
variableSetClient,
)

if runParams.EnvID != "" {
runParams.VarsID = runParams.EnvID
}

runParams := runner.RunOptions{
ID: runParams.ID,
DefinitionFile: runParams.DefinitionFile,
Expand Down Expand Up @@ -67,6 +71,12 @@ func init() {
runCmd.Flags().BoolVarP(&runParams.SkipResultWait, "skip-result-wait", "W", false, "do not wait for results. exit immediately after test run started")
runCmd.Flags().StringVarP(&runParams.JUnitOuptutFile, "junit", "j", "", "file path to save test results in junit format")
runCmd.Flags().StringSliceVar(&runParams.RequriedGates, "required-gates", []string{}, "override default required gate. "+validRequiredGatesMsg())

//deprecated
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still supporting this in case users need it

runCmd.Flags().StringVarP(&runParams.EnvID, "env", "e", "", "environment file or ID to be used")
runCmd.Flags().MarkDeprecated("env", "use --vars instead")
runCmd.Flags().MarkShorthandDeprecated("e", "use --vars instead")

rootCmd.AddCommand(runCmd)
}

Expand All @@ -83,6 +93,7 @@ type runParameters struct {
ID string
DefinitionFile string
VarsID string
EnvID string
SkipResultWait bool
JUnitOuptutFile string
RequriedGates []string
Expand Down
17 changes: 15 additions & 2 deletions cli/cmd/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ var (
var (
httpClient = &resourcemanager.HTTPClient{}

variableSetClient = resourcemanager.NewClient(
variableSetPreprocessor = preprocessor.VariableSet(cliLogger)
variableSetClient = resourcemanager.NewClient(
httpClient, cliLogger,
"variableset", "variablesets",
resourcemanager.WithTableConfig(resourcemanager.TableConfig{
Expand All @@ -46,6 +47,8 @@ var (
},
}),
resourcemanager.WithResourceType("VariableSet"),
resourcemanager.WithApplyPreProcessor(variableSetPreprocessor.Preprocess),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preprocessor to switch Environment to VariableSet and DeprecatedAlias to pass the resourceType validations

resourcemanager.WithDeprecatedAlias("Environment"),
)

testPreprocessor = preprocessor.Test(cliLogger)
Expand Down Expand Up @@ -121,6 +124,13 @@ var (
resourcemanager.WithApplyPreProcessor(transactionPreprocessor.Preprocess),
)

// deprecated resources
deprecatedEnvironmentClient = resourcemanager.NewClient(
httpClient, cliLogger,
"environment", "environments",
resourcemanager.WithProxyResource("variableset"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used to create an entry in the registry that will point to the variable set client

)

resources = resourcemanager.NewRegistry().
Register(
resourcemanager.NewClient(
Expand Down Expand Up @@ -231,7 +241,10 @@ var (
).
Register(variableSetClient).
Register(transactionClient).
Register(testClient)
Register(testClient).

// deprecated resources
Register(deprecatedEnvironmentClient)
)

func resourceList() string {
Expand Down
2 changes: 1 addition & 1 deletion cli/pkg/resourcemanager/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (c Client) validType(inputFile fileutil.File) error {
}
c.logger.Debug("Parsed type", zap.String("type", t))

if t != c.resourceType() {
if t != c.resourceType() && t != c.options.deprecatedAlias {
return fmt.Errorf("cannot apply %s to %s resource", t, c.resourceType())
}

Expand Down
1 change: 1 addition & 0 deletions cli/pkg/resourcemanager/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func (c Client) resourceType() string {
if c.options.resourceType != "" {
return c.options.resourceType
}

// language.Und means Undefined
caser := cases.Title(language.Und, cases.NoLower)
return caser.String(c.resourceName)
Expand Down
14 changes: 14 additions & 0 deletions cli/pkg/resourcemanager/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ type options struct {
tableConfig TableConfig
deleteSuccessMsg string
resourceType string
deprecatedAlias string
proxyResource string
}

type option func(*options)
Expand Down Expand Up @@ -32,3 +34,15 @@ func WithResourceType(resourceType string) option {
o.resourceType = resourceType
}
}

func WithDeprecatedAlias(resourceType string) option {
return func(o *options) {
o.deprecatedAlias = resourceType
}
}

func WithProxyResource(proxyResource string) option {
return func(o *options) {
o.proxyResource = proxyResource
}
}
25 changes: 23 additions & 2 deletions cli/pkg/resourcemanager/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package resourcemanager

import (
"errors"
"fmt"
"sort"

"github.com/agnivade/levenshtein"
Expand Down Expand Up @@ -30,13 +31,33 @@ func (r *Registry) Get(resourceName string) (Client, error) {
return Client{}, ErrResourceNotFound
}

if c.options.proxyResource != "" {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If proxy, check if the proxied resource exists

c.logger.Warn(fmt.Sprintf("The resource `%s` is deprecated and will be removed in a future version. Please use `%s` instead.", c.resourceName, c.options.proxyResource))
return r.Get(c.options.proxyResource)
}

return c, nil
}

func (r *Registry) Exists(resourceName string) bool {
c, ok := r.resources[resourceName]
if !ok {
return false
}

if c.options.proxyResource != "" {
return r.Exists(c.options.proxyResource)
}

return true
}

func (r *Registry) List() []string {
var resources []string
for k := range r.resources {
resources = append(resources, k)
for k, c := range r.resources {
if c.options.proxyResource == "" {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting the proxy resources from the list

resources = append(resources, k)
}
}

sort.Strings(resources)
Expand Down
45 changes: 45 additions & 0 deletions cli/preprocessor/variableset.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package preprocessor

import (
"context"
"fmt"

"github.com/goccy/go-yaml"
"github.com/kubeshop/tracetest/cli/pkg/fileutil"
"go.uber.org/zap"
)

type variableSet struct {
logger *zap.Logger
}

type generic struct {
Type string `yaml:"type"`
Spec interface{} `yaml:"spec"`
}

func VariableSet(logger *zap.Logger) variableSet {
return variableSet{
logger: logger,
}
}

func (vs variableSet) Preprocess(ctx context.Context, input fileutil.File) (fileutil.File, error) {
var resource generic
err := yaml.Unmarshal(input.Contents(), &resource)
if err != nil {
vs.logger.Error("error parsing test", zap.String("content", string(input.Contents())), zap.Error(err))
return input, fmt.Errorf("could not unmarshal test yaml: %w", err)
}

if resource.Type == "Environment" {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updates Environment to VariableSet

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to add a //TODO comment saying that this mapping will be removed in future versions?

resource.Type = "VariableSet"
}

marshalled, err := yaml.Marshal(resource)
if err != nil {
return input, fmt.Errorf("could not marshal test yaml: %w", err)
}

return fileutil.New(input.AbsPath(), marshalled), nil
}
9 changes: 0 additions & 9 deletions cli/testrunner.yaml

This file was deleted.

9 changes: 9 additions & 0 deletions testing/cli-e2etest/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ TRACETEST_CLI?="${PROJECT_ROOT}/../../dist/tracetest"
TEST_ENVIRONMENT?="jaeger"
TAG?="dev"
ENABLE_CLI_DEBUG?="false"
TEST_SCENARIO?=test

help: Makefile ## show list of commands
@echo "Choose a command run:"
Expand All @@ -17,6 +18,14 @@ test: # run tests for this application
go clean -testcache; \
go test -v -timeout 300s -p 1 ./...;

test/scenario: # run tests for this application
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will allows us execute a single testscenario

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! 🎉

export TRACETEST_CLI=$(TRACETEST_CLI); \
export TEST_ENVIRONMENT=$(TEST_ENVIRONMENT); \
export TAG=$(TAG); \
export ENABLE_CLI_DEBUG=$(ENABLE_CLI_DEBUG); \
go clean -testcache; \
go test -v -timeout 300s -p 1 "$(PROJECT_ROOT)/testscenarios/$(TEST_SCENARIO)";

test/debug: # run tests for this application with debug mode enabled
export ENABLE_CLI_DEBUG="true"; \
make test;
36 changes: 18 additions & 18 deletions testing/cli-e2etest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ The main idea is to test every CLI command against the Tracetest server with dif
| CLI Command | Test scenarios |
| ------------------------------------------------------------------ | -------------- |
| `run test -f [test-definition]` | [RunTestWithGrpcTrigger](./testscenarios/test/run_test_with_grpc_trigger_test.go) |
| `run test -f [test-definition] -e [environment-id]` | [RunTestWithHttpTriggerAndEnvironmentFile](./testscenarios/test/run_test_with_http_trigger_and_environment_file_test.go) |
| `run test -f [test-definition] -e [environment-definition]` | [RunTestWithHttpTriggerAndEnvironmentFile](./testscenarios/test/run_test_with_http_trigger_and_environment_file_test.go) |
| `run test -f [test-definition] --vars [variableset-id]` | [RunTestWithHttpTriggerAndVariableSetFile](./testscenarios/test/run_test_with_http_trigger_and_variableset_file_test.go) |
| `run test -f [test-definition] --vars [variableset-definition]` | [RunTestWithHttpTriggerAndVariableSetFile](./testscenarios/test/run_test_with_http_trigger_and_variableset_file_test.go) |
| `run transaction -f [transaction-definition]` | [RunTransaction](./testscenarios/transaction//run_transaction_test.go) |
| `run transaction -f [transaction-definition] -e [environment-id]` | |
| `run transaction -f [transaction-definition] -e [environment-definition]` | |
| `run transaction -f [transaction-definition] --vars [variableset-id]` | |
| `run transaction -f [transaction-definition] --vars [variableset-definition]` | |

### Resources: Config

Expand Down Expand Up @@ -81,23 +81,23 @@ The main idea is to test every CLI command against the Tracetest server with dif
| `list demo --skip 1 --take 1` | [ListDemo](./testscenarios/demo/list_demos_test.go) |
| `list demo --sortBy name --sortDirection asc` | [ListDemo](./testscenarios/demo/list_demos_test.go) |

### Resources: Environment
### Resources: VariableSet

| CLI Command | Test scenarios |
| ----------------------------------------------------------- | -------------- |
| `apply environment -f [new-environment-file]` | [ApplyEnvironment](./testscenarios/environment/apply_environment_test.go) |
| `apply environment -f [existing-environment-file]` | [ApplyEnvironment](./testscenarios/environment/apply_environment_test.go) |
| `delete environment --id [existing-id]` | [DeleteEnvironment](./testscenarios/environment/delete_environment_test.go) |
| `delete environment --id [non-existing-id]` | [DeleteEnvironment](./testscenarios/environment/delete_environment_test.go) |
| `get environment --id [non-existing-id]` | [GetEnvironment](./testscenarios/environment/get_environment_test.go), [DeleteEnvironment](./testscenarios/environment/delete_environment_test.go) |
| `get environment --id [existing-id] --output pretty` | [GetEnvironment](./testscenarios/environment/get_environment_test.go) |
| `get environment --id [existing-id] --output json` | [GetEnvironment](./testscenarios/environment/get_environment_test.go) |
| `get environment --id [existing-id] --output yaml` | [GetEnvironment](./testscenarios/environment/get_environment_test.go) |
| `list environment --output pretty` | [ListEnvironment](./testscenarios/environment/list_environments_test.go) |
| `list environment --output json` | [ListEnvironment](./testscenarios/environment/list_environments_test.go) |
| `list environment --output yaml` | [ListEnvironment](./testscenarios/environment/list_environments_test.go) |
| `list environment --skip 1 --take 2` | [ListEnvironment](./testscenarios/environment/list_environments_test.go) |
| `list environment --sortBy name --sortDirection asc` | [ListEnvironment](./testscenarios/environment/list_environments_test.go) |
| `apply variableset -f [new-variableset-file]` | [ApplyVariableSet](./testscenarios/variableset/apply_variableset_test.go) |
| `apply variableset -f [existing-variableset-file]` | [ApplyVariableSet](./testscenarios/variableset/apply_variableset_test.go) |
| `delete variableset --id [existing-id]` | [DeleteVariableSet](./testscenarios/variableset/delete_variableset_test.go) |
| `delete variableset --id [non-existing-id]` | [DeleteVariableSet](./testscenarios/variableset/delete_variableset_test.go) |
| `get variableset --id [non-existing-id]` | [GetVariableSet](./testscenarios/variableset/get_variableset_test.go), [DeleteVariableSet](./testscenarios/variableset/delete_variableset_test.go) |
| `get variableset --id [existing-id] --output pretty` | [GetVariableSet](./testscenarios/variableset/get_variableset_test.go) |
| `get variableset --id [existing-id] --output json` | [GetVariableSet](./testscenarios/variableset/get_variableset_test.go) |
| `get variableset --id [existing-id] --output yaml` | [GetVariableSet](./testscenarios/variableset/get_variableset_test.go) |
| `list variableset --output pretty` | [ListVariableSet](./testscenarios/variableset/list_variableset_test.go) |
| `list variableset --output json` | [ListVariableSet](./testscenarios/variableset/list_variableset_test.go) |
| `list variableset --output yaml` | [ListVariableSet](./testscenarios/variableset/list_variableset_test.go) |
| `list variableset --skip 1 --take 2` | [ListVariableSet](./testscenarios/variableset/list_variableset_test.go) |
| `list variableset --sortBy name --sortDirection asc` | [ListVariableSet](./testscenarios/variableset/list_variableset_test.go) |

### Resources: PollingProfile

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
type: Environment
spec:
id: deprecated-pokeapi-env
name: deprecated-pokeapi-env
values:
- key: POKEMON_NAME
value: snorlax
- key: POKEMON_URL
value: https://assets.pokemon.com/assets/cms2/img/pokedex/full/143.png
27 changes: 27 additions & 0 deletions testing/cli-e2etest/testscenarios/test/run_test_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,33 @@ func TestRunTestWithHttpTriggerAndVariableSetFile(t *testing.T) {
require.Equal("https://assets.pokemon.com/assets/cms2/img/pokedex/full/143.png", environmentVars.Spec.Values[1].Value)
})

t.Run("should pass when using the deprecated environment definition file", func(t *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a test to validate that the old way still works as expected

result := tracetestcli.Exec(t, "get environment --id deprecated-pokeapi-env", tracetestcli.WithCLIConfig(cliConfig))
helpers.RequireExitCodeEqual(t, result, 0)
require.Contains(result.StdOut, "The resource `environment` is deprecated and will be removed in a future version. Please use `variableset` instead.")
require.Contains(result.StdOut, "Resource variableset with ID deprecated-pokeapi-env not found")

// When I try to run a test with a http trigger and a variable set file
// Then it should pass
environmentFile := env.GetTestResourcePath(t, "deprecated-environment")
testFile := env.GetTestResourcePath(t, "http-trigger-with-environment-file")

command := fmt.Sprintf("run test -f %s --env %s", testFile, environmentFile)
result = tracetestcli.Exec(t, command, tracetestcli.WithCLIConfig(cliConfig))
helpers.RequireExitCodeEqual(t, result, 0)
require.Contains(result.StdOut, "✔ It should add a Pokemon correctly")
require.Contains(result.StdOut, "✔ It should save the correct data")

// When I try to get the variable set created on the previous step
// Then it should retrieve it correctly
result = tracetestcli.Exec(t, "get environment --id deprecated-pokeapi-env", tracetestcli.WithCLIConfig(cliConfig))
helpers.RequireExitCodeEqual(t, result, 0)

require.Contains(result.StdOut, "The resource `environment` is deprecated and will be removed in a future version. Please use `variableset` instead.")
require.Contains(result.StdOut, "VariableSet")
require.Contains(result.StdOut, "https://assets.pokemon.com/assets/cms2/img/pokedex/full/143.png")
})

t.Run("should pass when using variable set id", func(t *testing.T) {
// Given I am a Tracetest CLI user
// And I have my server recently created
Expand Down
Loading
Loading