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.

Sign up for GitHub

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

✨ Add support for regexp in profile activation kubeContext #2065

Merged
merged 5 commits into from
Jul 5, 2019
Merged

✨ Add support for regexp in profile activation kubeContext #2065

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af3de31
:sparkles: Add support for regexp in profile activation kubeContext
charlyx May 3, 2019
46e5cd1
Remove bad test
charlyx May 6, 2019
047e154
Update pkg/skaffold/schema/profiles.go
charlyx May 10, 2019
33b63ae
:sparkles: Add support for `! ` prefix with regexp
charlyx May 10, 2019
f0782e7
:memo: Update documentation for kubeContext regexp support
charlyx May 10, 2019
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
9 changes: 8 additions & 1 deletion pkg/skaffold/schema/profiles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"os"
"reflect"
re "regexp"
"strings"

cfg "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
Expand Down Expand Up @@ -124,7 +125,13 @@ func satisfies(expected, actual string) bool {
if strings.HasPrefix(expected, "!") {
return actual != expected[1:]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should support the ! prefix for regular expressions too, e.g.:

!dev-.* should activate on all that don't match it

}
return actual == expected
matcher, err := re.Compile(expected)
if err != nil {
logrus.Infof("Not a regexp: %s, falling back to string equals", expected)
charlyx marked this conversation as resolved.
Show resolved Hide resolved
return actual == expected
}

return matcher.MatchString(actual)
}

func applyProfile(config *latest.SkaffoldConfig, profile latest.Profile) error {
Expand Down
4 changes: 3 additions & 1 deletion pkg/skaffold/schema/profiles_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,10 @@ func TestActivatedProfiles(t *testing.T) {
{Name: "activated", Activation: []latest.Activation{{KubeContext: "prod-context"}}},
{Name: "not-activated", Activation: []latest.Activation{{KubeContext: "dev-context"}}},
{Name: "also-activated", Activation: []latest.Activation{{KubeContext: "!dev-context"}}},
{Name: "activated-regexp", Activation: []latest.Activation{{KubeContext: "prod-.*"}}},
{Name: "not-activated-regexp", Activation: []latest.Activation{{KubeContext: "dev-.*"}}},
Copy link
Contributor

Choose a reason for hiding this comment

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

we should cover the !(regex) and invalid regex cases too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I create another profile so I can test those cases more easily?

For instance, in order to test the invalid regex I would need a profile that matches when comparing strings, I guess. Also, is there any character limitation for the profile? I couldn't find any in create_context.go

Regarding the !regexp I'm not sure how to test it... Shall we assume that if the regexp is valid we're testing the regexp matching and not the string comparison so I can make a simple test?

Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to add as many profiles as needed. Also, simple tests are better!

},
expected: []string{"activated", "also-activated"},
expected: []string{"activated", "also-activated", "activated-regexp"},
}, {
description: "AND between activation criteria",
opts: &cfg.SkaffoldOptions{
Expand Down