diff --git a/lib/services/github_test.go b/lib/services/github_test.go index 9f2c65ea74e44..13e96322d89a9 100644 --- a/lib/services/github_test.go +++ b/lib/services/github_test.go @@ -17,15 +17,17 @@ limitations under the License. package services import ( + "testing" + "github.com/gravitational/teleport/api/types" - check "gopkg.in/check.v1" -) -type GithubSuite struct{} + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" +) -var _ = check.Suite(&GithubSuite{}) +func TestUnmarshal(t *testing.T) { + t.Parallel() -func (g *GithubSuite) TestUnmarshal(c *check.C) { data := []byte(`{"kind": "github", "version": "v3", "metadata": { @@ -43,7 +45,7 @@ func (g *GithubSuite) TestUnmarshal(c *check.C) { }] }}`) connector, err := UnmarshalGithubConnector(data) - c.Assert(err, check.IsNil) + require.NoError(t, err) expected, err := types.NewGithubConnector("github", types.GithubConnectorSpecV3{ ClientID: "aaa", ClientSecret: "bbb", @@ -57,11 +59,13 @@ func (g *GithubSuite) TestUnmarshal(c *check.C) { }, }, }) - c.Assert(err, check.IsNil) - c.Assert(expected, check.DeepEquals, connector) + require.NoError(t, err) + require.Empty(t, cmp.Diff(expected, connector)) } -func (g *GithubSuite) TestMapClaims(c *check.C) { +func TestMapClaims(t *testing.T) { + t.Parallel() + connector, err := types.NewGithubConnector("github", types.GithubConnectorSpecV3{ ClientID: "aaa", ClientSecret: "bbb", @@ -90,16 +94,16 @@ func (g *GithubSuite) TestMapClaims(c *check.C) { }, }, }) - c.Assert(err, check.IsNil) + require.NoError(t, err) roles, kubeGroups, kubeUsers := connector.MapClaims(types.GithubClaims{ OrganizationToTeams: map[string][]string{ "gravitational": {"admins"}, }, }) - c.Assert(roles, check.DeepEquals, []string{"admin", "dev", "system"}) - c.Assert(kubeGroups, check.DeepEquals, []string{"system:masters", "kube-devs"}) - c.Assert(kubeUsers, check.DeepEquals, []string{"alice@example.com"}) + require.Empty(t, cmp.Diff(roles, []string{"admin", "dev", "system"})) + require.Empty(t, cmp.Diff(kubeGroups, []string{"system:masters", "kube-devs"})) + require.Empty(t, cmp.Diff(kubeUsers, []string{"alice@example.com"})) roles, kubeGroups, kubeUsers = connector.MapClaims(types.GithubClaims{ OrganizationToTeams: map[string][]string{ @@ -107,16 +111,16 @@ func (g *GithubSuite) TestMapClaims(c *check.C) { }, }) - c.Assert(roles, check.DeepEquals, []string{"dev", "test"}) - c.Assert(kubeGroups, check.DeepEquals, []string{"kube-devs"}) - c.Assert(kubeUsers, check.DeepEquals, []string(nil)) + require.Empty(t, cmp.Diff(roles, []string{"dev", "test"})) + require.Empty(t, cmp.Diff(kubeGroups, []string{"kube-devs"})) + require.Empty(t, cmp.Diff(kubeUsers, []string(nil))) roles, kubeGroups, kubeUsers = connector.MapClaims(types.GithubClaims{ OrganizationToTeams: map[string][]string{ "gravitational": {"admins", "devs"}, }, }) - c.Assert(roles, check.DeepEquals, []string{"admin", "dev", "test", "system"}) - c.Assert(kubeGroups, check.DeepEquals, []string{"system:masters", "kube-devs"}) - c.Assert(kubeUsers, check.DeepEquals, []string{"alice@example.com"}) + require.Empty(t, cmp.Diff(roles, []string{"admin", "dev", "test", "system"})) + require.Empty(t, cmp.Diff(kubeGroups, []string{"system:masters", "kube-devs"})) + require.Empty(t, cmp.Diff(kubeUsers, []string{"alice@example.com"})) } diff --git a/lib/services/license_test.go b/lib/services/license_test.go index 46c7a402c3db4..ce68aad58283c 100644 --- a/lib/services/license_test.go +++ b/lib/services/license_test.go @@ -17,20 +17,20 @@ limitations under the License. package services import ( - "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/fixtures" + "fmt" + "testing" - "gopkg.in/check.v1" + "github.com/gravitational/teleport/api/types" "github.com/gravitational/trace" -) -type LicenseSuite struct { -} + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" +) -var _ = check.Suite(&LicenseSuite{}) +func TestLicenseUnmarshal(t *testing.T) { + t.Parallel() -func (l *LicenseSuite) TestUnmarshal(c *check.C) { type testCase struct { description string input string @@ -103,18 +103,18 @@ func (l *LicenseSuite) TestUnmarshal(c *check.C) { }, } for _, tc := range testCases { - comment := check.Commentf("test case %q", tc.description) + comment := fmt.Sprintf("test case %q", tc.description) out, err := UnmarshalLicense([]byte(tc.input)) if tc.err == nil { - c.Assert(err, check.IsNil, comment) - fixtures.DeepCompare(c, tc.expected, out) + require.NoError(t, err, comment) + require.Empty(t, cmp.Diff(tc.expected, out)) data, err := MarshalLicense(out) - c.Assert(err, check.IsNil, comment) + require.NoError(t, err, comment) out2, err := UnmarshalLicense(data) - c.Assert(err, check.IsNil, comment) - fixtures.DeepCompare(c, tc.expected, out2) + require.NoError(t, err, comment) + require.Empty(t, cmp.Diff(tc.expected, out2)) } else { - c.Assert(err, check.FitsTypeOf, tc.err, comment) + require.IsType(t, err, tc.err, comment) } } } diff --git a/lib/services/local/resource_test.go b/lib/services/local/resource_test.go index d2630d2b427a9..c42910d866fe1 100644 --- a/lib/services/local/resource_test.go +++ b/lib/services/local/resource_test.go @@ -27,124 +27,85 @@ import ( apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/backend" - "github.com/gravitational/teleport/lib/backend/memory" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/services/suite" + "github.com/stretchr/testify/require" "github.com/jonboulle/clockwork" - "gopkg.in/check.v1" ) -func Test(t *testing.T) { check.TestingT(t) } +func TestUserResource(t *testing.T) { + t.Parallel() -type ResourceSuite struct { - bk backend.Backend + tt := setupServicesContext(context.Background(), t) + runUserResourceTest(t, tt, false) } -var _ = check.Suite(&ResourceSuite{}) +func TestUserResourceWithSecrets(t *testing.T) { + t.Parallel() -func (r *ResourceSuite) SetUpTest(c *check.C) { - var err error - - clock := clockwork.NewFakeClockAt(time.Now()) - - r.bk, err = memory.New(memory.Config{ - Context: context.Background(), - Clock: clock, - }) - c.Assert(err, check.IsNil) -} - -func (r *ResourceSuite) TearDownTest(c *check.C) { - c.Assert(r.bk.Close(), check.IsNil) -} - -func (r *ResourceSuite) dumpResources(c *check.C) []types.Resource { - startKey := []byte("/") - endKey := backend.RangeEnd(startKey) - result, err := r.bk.GetRange(context.TODO(), startKey, endKey, 0) - c.Assert(err, check.IsNil) - resources, err := ItemsToResources(result.Items...) - c.Assert(err, check.IsNil) - return resources -} - -func (r *ResourceSuite) runCreationChecks(c *check.C, resources ...types.Resource) { - for _, rsc := range resources { - switch r := rsc.(type) { - case types.User: - c.Logf("Creating User: %+v", r) - default: - } - } - err := CreateResources(context.TODO(), r.bk, resources...) - c.Assert(err, check.IsNil) - dump := r.dumpResources(c) -Outer: - for _, exp := range resources { - for _, got := range dump { - if got.GetKind() == exp.GetKind() && got.GetName() == exp.GetName() && got.Expiry() == exp.Expiry() { - continue Outer - } - } - c.Errorf("Missing expected resource kind=%s,name=%s,expiry=%v", exp.GetKind(), exp.GetName(), exp.Expiry().String()) - } + tt := setupServicesContext(context.Background(), t) + runUserResourceTest(t, tt, true) } -func (r *ResourceSuite) TestUserResource(c *check.C) { - r.runUserResourceTest(c, false) -} - -func (r *ResourceSuite) TestUserResourceWithSecrets(c *check.C) { - r.runUserResourceTest(c, true) -} +func runUserResourceTest(t *testing.T, tt *servicesContext, withSecrets bool) { + expiry := tt.bk.Clock().Now().Add(time.Minute) -func (r *ResourceSuite) runUserResourceTest(c *check.C, withSecrets bool) { - expiry := r.bk.Clock().Now().Add(time.Minute) + alice := newUserTestCase(t, "alice", nil, withSecrets, expiry) + bob := newUserTestCase(t, "bob", nil, withSecrets, expiry) - alice := newUserTestCase(c, "alice", nil, withSecrets, expiry) - bob := newUserTestCase(c, "bob", nil, withSecrets, expiry) // Check basic dynamic item creation - r.runCreationChecks(c, alice, bob) + runCreationChecks(t, tt, alice, bob) + // Check that dynamically created item is compatible with service - s := NewIdentityService(r.bk) + s := NewIdentityService(tt.bk) b, err := s.GetUser("bob", withSecrets) - c.Assert(err, check.IsNil) - c.Assert(services.UsersEquals(bob, b), check.Equals, true, check.Commentf("dynamically inserted user does not match")) + require.NoError(t, err) + require.Equal(t, services.UsersEquals(bob, b), true, "dynamically inserted user does not match") allUsers, err := s.GetUsers(withSecrets) - c.Assert(err, check.IsNil) - c.Assert(len(allUsers), check.Equals, 2, check.Commentf("expected exactly two users")) + require.NoError(t, err) + require.Equal(t, len(allUsers), 2, "expected exactly two users") for _, user := range allUsers { switch user.GetName() { case "alice": - c.Assert(services.UsersEquals(alice, user), check.Equals, true, check.Commentf("alice does not match")) + require.Equal(t, services.UsersEquals(alice, user), true, "alice does not match") case "bob": - c.Assert(services.UsersEquals(bob, user), check.Equals, true, check.Commentf("bob does not match")) + require.Equal(t, services.UsersEquals(bob, user), true, "bob does not match") default: - c.Errorf("Unexpected user %q", user.GetName()) + t.Errorf("Unexpected user %q", user.GetName()) } } // Advance the clock to let the users to expire. - r.bk.Clock().(clockwork.FakeClock).Advance(2 * time.Minute) + tt.bk.Clock().(clockwork.FakeClock).Advance(2 * time.Minute) allUsers, err = s.GetUsers(withSecrets) - c.Assert(err, check.IsNil) - c.Assert(len(allUsers), check.Equals, 0, check.Commentf("expected all users to expire")) + require.NoError(t, err) + require.Equal(t, len(allUsers), 0, "expected all users to expire") } -func (r *ResourceSuite) TestCertAuthorityResource(c *check.C) { +func TestCertAuthorityResource(t *testing.T) { + t.Parallel() + + tt := setupServicesContext(context.Background(), t) + userCA := suite.NewTestCA(types.UserCA, "example.com") hostCA := suite.NewTestCA(types.HostCA, "example.com") + // Check basic dynamic item creation - r.runCreationChecks(c, userCA, hostCA) + runCreationChecks(t, tt, userCA, hostCA) + // Check that dynamically created item is compatible with service - s := NewCAService(r.bk) + s := NewCAService(tt.bk) err := s.CompareAndSwapCertAuthority(userCA, userCA) - c.Assert(err, check.IsNil) + require.NoError(t, err) } -func (r *ResourceSuite) TestTrustedClusterResource(c *check.C) { +func TestTrustedClusterResource(t *testing.T) { + t.Parallel() + ctx := context.Background() + tt := setupServicesContext(ctx, t) + foo, err := types.NewTrustedCluster("foo", types.TrustedClusterSpecV2{ Enabled: true, Roles: []string{"bar", "baz"}, @@ -152,7 +113,7 @@ func (r *ResourceSuite) TestTrustedClusterResource(c *check.C) { ProxyAddress: "quux", ReverseTunnelAddress: "quuz", }) - c.Assert(err, check.IsNil) + require.NoError(t, err) bar, err := types.NewTrustedCluster("bar", types.TrustedClusterSpecV2{ Enabled: false, @@ -161,19 +122,24 @@ func (r *ResourceSuite) TestTrustedClusterResource(c *check.C) { ProxyAddress: "quuz", ReverseTunnelAddress: "corge", }) - c.Assert(err, check.IsNil) + require.NoError(t, err) + // Check basic dynamic item creation - r.runCreationChecks(c, foo, bar) + runCreationChecks(t, tt, foo, bar) - s := NewPresenceService(r.bk) + s := NewPresenceService(tt.bk) _, err = s.GetTrustedCluster(ctx, "foo") - c.Assert(err, check.IsNil) + require.NoError(t, err) _, err = s.GetTrustedCluster(ctx, "bar") - c.Assert(err, check.IsNil) + require.NoError(t, err) } -func (r *ResourceSuite) TestGithubConnectorResource(c *check.C) { +func TestGithubConnectorResource(t *testing.T) { + t.Parallel() + ctx := context.Background() + tt := setupServicesContext(ctx, t) + connector := &types.GithubConnectorV3{ Kind: types.KindGithubConnector, Version: types.V3, @@ -196,28 +162,29 @@ func (r *ResourceSuite) TestGithubConnectorResource(c *check.C) { }, }, } + // Check basic dynamic item creation - r.runCreationChecks(c, connector) + runCreationChecks(t, tt, connector) - s := NewIdentityService(r.bk) + s := NewIdentityService(tt.bk) _, err := s.GetGithubConnector(ctx, "github", true) - c.Assert(err, check.IsNil) + require.NoError(t, err) } -func localAuthSecretsTestCase(c *check.C) types.LocalAuthSecrets { +func localAuthSecretsTestCase(t *testing.T) types.LocalAuthSecrets { var auth types.LocalAuthSecrets var err error auth.PasswordHash, err = bcrypt.GenerateFromPassword([]byte("insecure"), bcrypt.MinCost) - c.Assert(err, check.IsNil) + require.NoError(t, err) dev, err := services.NewTOTPDevice("otp", base32.StdEncoding.EncodeToString([]byte("abc123")), time.Now()) - c.Assert(err, check.IsNil) + require.NoError(t, err) auth.MFA = append(auth.MFA, dev) return auth } -func newUserTestCase(c *check.C, name string, roles []string, withSecrets bool, expires time.Time) types.User { +func newUserTestCase(t *testing.T, name string, roles []string, withSecrets bool, expires time.Time) types.User { user := types.UserV2{ Kind: types.KindUser, Version: types.V2, @@ -231,8 +198,40 @@ func newUserTestCase(c *check.C, name string, roles []string, withSecrets bool, }, } if withSecrets { - auth := localAuthSecretsTestCase(c) + auth := localAuthSecretsTestCase(t) user.SetLocalAuth(&auth) } return &user } + +func dumpResources(t *testing.T, tt *servicesContext) []types.Resource { + startKey := []byte("/") + endKey := backend.RangeEnd(startKey) + result, err := tt.bk.GetRange(context.TODO(), startKey, endKey, 0) + require.NoError(t, err) + resources, err := ItemsToResources(result.Items...) + require.NoError(t, err) + return resources +} + +func runCreationChecks(t *testing.T, tt *servicesContext, resources ...types.Resource) { + for _, rsc := range resources { + switch r := rsc.(type) { + case types.User: + t.Logf("Creating User: %+v", r) + default: + } + } + err := CreateResources(context.TODO(), tt.bk, resources...) + require.NoError(t, err) + dump := dumpResources(t, tt) +Outer: + for _, exp := range resources { + for _, got := range dump { + if got.GetKind() == exp.GetKind() && got.GetName() == exp.GetName() && got.Expiry() == exp.Expiry() { + continue Outer + } + } + t.Errorf("Missing expected resource kind=%s,name=%s,expiry=%v", exp.GetKind(), exp.GetName(), exp.Expiry().String()) + } +} diff --git a/lib/services/map_test.go b/lib/services/map_test.go index eed603001c737..8d10bd2ef1dc6 100644 --- a/lib/services/map_test.go +++ b/lib/services/map_test.go @@ -17,19 +17,20 @@ limitations under the License. package services import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/types" - - "gopkg.in/check.v1" + "github.com/stretchr/testify/require" "github.com/gravitational/trace" ) -type RoleMapSuite struct{} +func TestRoleParsing(t *testing.T) { + t.Parallel() -var _ = check.Suite(&RoleMapSuite{}) - -func (s *RoleMapSuite) TestRoleParsing(c *check.C) { testCases := []struct { roleMap types.RoleMap err error @@ -64,18 +65,20 @@ func (s *RoleMapSuite) TestRoleParsing(c *check.C) { } for i, tc := range testCases { - comment := check.Commentf("test case '%v'", i) + comment := fmt.Sprintf("test case '%v'", i) _, err := parseRoleMap(tc.roleMap) if tc.err != nil { - c.Assert(err, check.NotNil, comment) - c.Assert(err, check.FitsTypeOf, tc.err) + require.NotNilf(t, err, comment) + require.IsTypef(t, err, tc.err, comment) } else { - c.Assert(err, check.IsNil) + require.NoError(t, err) } } } -func (s *RoleMapSuite) TestRoleMap(c *check.C) { +func TestRoleMap(t *testing.T) { + t.Parallel() + testCases := []struct { remote []string local []string @@ -183,14 +186,14 @@ func (s *RoleMapSuite) TestRoleMap(c *check.C) { } for _, tc := range testCases { - comment := check.Commentf("test case '%v'", tc.name) + comment := fmt.Sprintf("test case '%v'", tc.name) local, err := MapRoles(tc.roleMap, tc.remote) if tc.err != nil { - c.Assert(err, check.NotNil, comment) - c.Assert(err, check.FitsTypeOf, tc.err) + require.NotNilf(t, err, comment) + require.IsTypef(t, err, tc.err, comment) } else { - c.Assert(err, check.IsNil, comment) - c.Assert(local, check.DeepEquals, tc.local, comment) + require.NoError(t, err, comment) + require.Empty(t, cmp.Diff(local, tc.local), comment) } } } diff --git a/lib/services/saml_test.go b/lib/services/saml_test.go index 26f31f680cf3c..2512e46b94e30 100644 --- a/lib/services/saml_test.go +++ b/lib/services/saml_test.go @@ -25,37 +25,37 @@ import ( "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/fixtures" + "github.com/google/go-cmp/cmp" "github.com/gravitational/trace" "github.com/stretchr/testify/require" - "gopkg.in/check.v1" kyaml "k8s.io/apimachinery/pkg/util/yaml" ) -type SAMLSuite struct{} - -var _ = check.Suite(&SAMLSuite{}) +func TestParseFromMetadata(t *testing.T) { + t.Parallel() -func (s *SAMLSuite) TestParseFromMetadata(c *check.C) { input := fixtures.SAMLOktaConnectorV2 decoder := kyaml.NewYAMLOrJSONDecoder(strings.NewReader(input), defaults.LookaheadBufSize) var raw UnknownResource err := decoder.Decode(&raw) - c.Assert(err, check.IsNil) + require.NoError(t, err) oc, err := UnmarshalSAMLConnector(raw.Raw) - c.Assert(err, check.IsNil) + require.NoError(t, err) err = ValidateSAMLConnector(oc, nil) - c.Assert(err, check.IsNil) - c.Assert(oc.GetIssuer(), check.Equals, "http://www.okta.com/exkafftca6RqPVgyZ0h7") - c.Assert(oc.GetSSO(), check.Equals, "https://dev-813354.oktapreview.com/app/gravitationaldev813354_teleportsaml_1/exkafftca6RqPVgyZ0h7/sso/saml") - c.Assert(oc.GetAssertionConsumerService(), check.Equals, "https://localhost:3080/v1/webapi/saml/acs") - c.Assert(oc.GetAudience(), check.Equals, "https://localhost:3080/v1/webapi/saml/acs") - c.Assert(oc.GetSigningKeyPair(), check.NotNil) - c.Assert(oc.GetAttributes(), check.DeepEquals, []string{"groups"}) + require.NoError(t, err) + require.Equal(t, oc.GetIssuer(), "http://www.okta.com/exkafftca6RqPVgyZ0h7") + require.Equal(t, oc.GetSSO(), "https://dev-813354.oktapreview.com/app/gravitationaldev813354_teleportsaml_1/exkafftca6RqPVgyZ0h7/sso/saml") + require.Equal(t, oc.GetAssertionConsumerService(), "https://localhost:3080/v1/webapi/saml/acs") + require.Equal(t, oc.GetAudience(), "https://localhost:3080/v1/webapi/saml/acs") + require.NotNil(t, oc.GetSigningKeyPair()) + require.Empty(t, cmp.Diff(oc.GetAttributes(), []string{"groups"})) } func TestCheckSAMLEntityDescriptor(t *testing.T) { + t.Parallel() + input := fixtures.SAMLOktaConnectorV2 decoder := kyaml.NewYAMLOrJSONDecoder(strings.NewReader(input), defaults.LookaheadBufSize) diff --git a/lib/services/servers_test.go b/lib/services/servers_test.go index 3504af55d3253..7669e472c1b75 100644 --- a/lib/services/servers_test.go +++ b/lib/services/servers_test.go @@ -24,20 +24,16 @@ import ( apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/fixtures" + "github.com/gravitational/trace" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" - "gopkg.in/check.v1" ) -type ServerSuite struct { -} - -var _ = check.Suite(&ServerSuite{}) - // TestServersCompare tests comparing two servers -func (s *ServerSuite) TestServersCompare(c *check.C) { +func TestServersCompare(t *testing.T) { + t.Parallel() + node := &types.ServerV2{ Kind: types.KindNode, Version: types.V2, @@ -54,42 +50,42 @@ func (s *ServerSuite) TestServersCompare(c *check.C) { } node.SetExpiry(time.Date(2018, 1, 2, 3, 4, 5, 6, time.UTC)) // Server is equal to itself - c.Assert(CompareServers(node, node), check.Equals, Equal) + require.Equal(t, CompareServers(node, node), Equal) // Only timestamps are different node2 := *node node2.SetExpiry(time.Date(2018, 1, 2, 3, 4, 5, 8, time.UTC)) - c.Assert(CompareServers(node, &node2), check.Equals, OnlyTimestampsDifferent) + require.Equal(t, CompareServers(node, &node2), OnlyTimestampsDifferent) // Labels are different node2 = *node node2.Metadata.Labels = map[string]string{"a": "d"} - c.Assert(CompareServers(node, &node2), check.Equals, Different) + require.Equal(t, CompareServers(node, &node2), Different) // Command labels are different node2 = *node node2.Spec.CmdLabels = map[string]types.CommandLabelV2{"a": {Period: types.Duration(time.Minute), Command: []string{"ls", "-lR"}}} - c.Assert(CompareServers(node, &node2), check.Equals, Different) + require.Equal(t, CompareServers(node, &node2), Different) // Address has changed node2 = *node node2.Spec.Addr = "localhost:3033" - c.Assert(CompareServers(node, &node2), check.Equals, Different) + require.Equal(t, CompareServers(node, &node2), Different) // Public addr has changed node2 = *node node2.Spec.PublicAddr = "localhost:3033" - c.Assert(CompareServers(node, &node2), check.Equals, Different) + require.Equal(t, CompareServers(node, &node2), Different) // Hostname has changed node2 = *node node2.Spec.Hostname = "luna2" - c.Assert(CompareServers(node, &node2), check.Equals, Different) + require.Equal(t, CompareServers(node, &node2), Different) // TeleportVersion has changed node2 = *node node2.Spec.Version = "5.0.0" - c.Assert(CompareServers(node, &node2), check.Equals, Different) + require.Equal(t, CompareServers(node, &node2), Different) // Rotation has changed node2 = *node @@ -106,17 +102,19 @@ func (s *ServerSuite) TestServersCompare(c *check.C) { Standby: time.Date(2018, 3, 4, 5, 6, 13, 8, time.UTC), }, } - c.Assert(CompareServers(node, &node2), check.Equals, Different) + require.Equal(t, CompareServers(node, &node2), Different) } // TestGuessProxyHostAndVersion checks that the GuessProxyHostAndVersion // correctly guesses the public address of the proxy (Teleport Cluster). -func (s *ServerSuite) TestGuessProxyHostAndVersion(c *check.C) { +func TestGuessProxyHostAndVersion(t *testing.T) { + t.Parallel() + // No proxies passed in. host, version, err := GuessProxyHostAndVersion(nil) - c.Assert(host, check.Equals, "") - c.Assert(version, check.Equals, "") - fixtures.ExpectNotFound(c, err) + require.Equal(t, host, "") + require.Equal(t, version, "") + require.True(t, trace.IsNotFound(err)) // No proxies have public address set. proxyA := types.ServerV2{} @@ -124,9 +122,9 @@ func (s *ServerSuite) TestGuessProxyHostAndVersion(c *check.C) { proxyA.Spec.Version = "test-A" host, version, err = GuessProxyHostAndVersion([]types.Server{&proxyA}) - c.Assert(host, check.Equals, fmt.Sprintf("%v:%v", proxyA.Spec.Hostname, defaults.HTTPListenPort)) - c.Assert(version, check.Equals, proxyA.Spec.Version) - c.Assert(err, check.IsNil) + require.Equal(t, host, fmt.Sprintf("%v:%v", proxyA.Spec.Hostname, defaults.HTTPListenPort)) + require.Equal(t, version, proxyA.Spec.Version) + require.NoError(t, err) // At least one proxy has public address set. proxyB := types.ServerV2{} @@ -134,12 +132,14 @@ func (s *ServerSuite) TestGuessProxyHostAndVersion(c *check.C) { proxyB.Spec.Version = "test-B" host, version, err = GuessProxyHostAndVersion([]types.Server{&proxyA, &proxyB}) - c.Assert(host, check.Equals, proxyB.Spec.PublicAddr) - c.Assert(version, check.Equals, proxyB.Spec.Version) - c.Assert(err, check.IsNil) + require.Equal(t, host, proxyB.Spec.PublicAddr) + require.Equal(t, version, proxyB.Spec.Version) + require.NoError(t, err) } func TestUnmarshalServerKubernetes(t *testing.T) { + t.Parallel() + // Regression test for // https://github.com/gravitational/teleport/issues/4862 // @@ -212,6 +212,8 @@ func TestUnmarshalServerKubernetes(t *testing.T) { // TestOnlyTimestampsDifferent tests that OnlyTimestampsDifferent is returned // after checking that whether KubernetesClusters and Apps are different. func TestOnlyTimestampsDifferent(t *testing.T) { + t.Parallel() + now := time.Now() later := now.Add(time.Minute) diff --git a/lib/services/services_test.go b/lib/services/services_test.go index 959c59d2ad471..c23f9772d1c21 100644 --- a/lib/services/services_test.go +++ b/lib/services/services_test.go @@ -23,12 +23,10 @@ import ( apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/fixtures" "github.com/gravitational/teleport/lib/utils" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" - "gopkg.in/check.v1" ) func TestMain(m *testing.M) { @@ -36,57 +34,56 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -type ServicesSuite struct { -} - -func TestServices(t *testing.T) { check.TestingT(t) } - -var _ = check.Suite(&ServicesSuite{}) - // TestOptions tests command options operations -func (s *ServicesSuite) TestOptions(c *check.C) { +func TestOptions(t *testing.T) { + t.Parallel() + // test empty scenario out := AddOptions(nil) - c.Assert(out, check.HasLen, 0) + require.Len(t, out, 0) // make sure original option list is not affected in := []MarshalOption{} out = AddOptions(in, WithResourceID(1)) - c.Assert(out, check.HasLen, 1) - c.Assert(in, check.HasLen, 0) + require.Len(t, out, 1) + require.Len(t, in, 0) cfg, err := CollectOptions(out) - c.Assert(err, check.IsNil) - c.Assert(cfg.ID, check.Equals, int64(1)) + require.NoError(t, err) + require.Equal(t, cfg.ID, int64(1)) // Add a couple of other parameters out = AddOptions(in, WithResourceID(2), WithVersion(types.V2)) - c.Assert(out, check.HasLen, 2) - c.Assert(in, check.HasLen, 0) + require.Len(t, out, 2) + require.Len(t, in, 0) cfg, err = CollectOptions(out) - c.Assert(err, check.IsNil) - c.Assert(cfg.ID, check.Equals, int64(2)) - c.Assert(cfg.Version, check.Equals, types.V2) + require.NoError(t, err) + require.Equal(t, cfg.ID, int64(2)) + require.Equal(t, cfg.Version, types.V2) } // TestCommandLabels tests command labels -func (s *ServicesSuite) TestCommandLabels(c *check.C) { +func TestCommandLabels(t *testing.T) { + t.Parallel() + var l CommandLabels out := l.Clone() - c.Assert(out, check.HasLen, 0) + require.Len(t, out, 0) label := &types.CommandLabelV2{Command: []string{"ls", "-l"}, Period: types.Duration(time.Second)} l = CommandLabels{"a": label} out = l.Clone() - c.Assert(out, check.HasLen, 1) - fixtures.DeepCompare(c, out["a"], label) + require.Len(t, out, 1) + require.Empty(t, cmp.Diff(out["a"], label)) // make sure it's not a shallow copy label.Command[0] = "/bin/ls" - c.Assert(label.Command[0], check.Not(check.Equals), out["a"].GetCommand()) + require.NotEqual(t, label.Command[0], out["a"].GetCommand()) } func TestLabelKeyValidation(t *testing.T) { + t.Parallel() + tts := []struct { label string ok bool @@ -107,6 +104,7 @@ func TestLabelKeyValidation(t *testing.T) { func TestServerDeepCopy(t *testing.T) { t.Parallel() + // setup now := time.Date(1984, time.April, 4, 0, 0, 0, 0, time.UTC) expires := now.Add(1 * time.Hour) diff --git a/lib/services/suite/presence_test.go b/lib/services/suite/presence_test.go index c7cffe11e46d2..0a193d0f15fbd 100644 --- a/lib/services/suite/presence_test.go +++ b/lib/services/suite/presence_test.go @@ -44,7 +44,7 @@ func TestServerLabels(t *testing.T) { }, Spec: types.ServerSpecV2{ CmdLabels: map[string]types.CommandLabelV2{ - "time": types.CommandLabelV2{ + "time": { Period: types.NewDuration(time.Second), Command: []string{"time"}, Result: "now", diff --git a/lib/services/traits.go b/lib/services/traits.go index 011f1486c5e80..f026fe603f567 100644 --- a/lib/services/traits.go +++ b/lib/services/traits.go @@ -18,6 +18,7 @@ package services import ( "fmt" + "regexp" "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" @@ -28,6 +29,10 @@ import ( log "github.com/sirupsen/logrus" ) +// maxMismatchedTraitValuesLogged indicates the maximum number of trait values (that do not match a +// certain expression) to be shown in the log +const maxMismatchedTraitValuesLogged = 100 + // TraitsToRoles maps the supplied traits to a list of teleport role names. // Returns the list of roles mapped from traits. // `warnings` optionally contains the list of warnings potentially interesting to the user. @@ -74,32 +79,57 @@ func TraitsToRoleMatchers(ms types.TraitMappingSet, traits map[string][]string) // traitsToRoles maps the supplied traits to teleport role names and passes them to a collector. func traitsToRoles(ms types.TraitMappingSet, traits map[string][]string, collect func(role string, expanded bool)) (warnings []string) { +TraitMappingLoop: for _, mapping := range ms { + var regexpIgnoreCase *regexp.Regexp + var regexp *regexp.Regexp + for traitName, traitValues := range traits { if traitName != mapping.Trait { continue } + var mismatched []string TraitLoop: for _, traitValue := range traitValues { for _, role := range mapping.Roles { + // this ensures that the case-insensitive regexp is compiled at most once, and only if strictly needed; + // after this if, regexpIgnoreCase must be non-nil + if regexpIgnoreCase == nil { + var err error + regexpIgnoreCase, err = utils.RegexpWithConfig(mapping.Value, utils.RegexpConfig{IgnoreCase: true}) + if err != nil { + warnings = append(warnings, fmt.Sprintf("case-insensitive expression %q is not a valid regexp", mapping.Value)) + continue TraitMappingLoop + } + } + // Run the initial replacement case-insensitively. Doing so will filter out all literal non-matches // but will match on case discrepancies. We do another case-sensitive match below to see if the // case is different - outRole, err := utils.ReplaceRegexpWithConfig(mapping.Value, role, traitValue, utils.RegexpConfig{IgnoreCase: true}) + outRole, err := utils.ReplaceRegexpWith(regexpIgnoreCase, role, traitValue) switch { case err != nil: - if trace.IsNotFound(err) { - log.WithError(err).Debugf("Failed to match expression %q, replace with: %q input: %q.", mapping.Value, role, traitValue) - } // this trait value clearly did not match, move on to another + mismatched = append(mismatched, traitValue) continue TraitLoop case outRole == "": case outRole != "": + // this ensures that the case-sensitive regexp is compiled at most once, and only if strictly needed; + // after this if, regexp must be non-nil + if regexp == nil { + var err error + regexp, err = utils.RegexpWithConfig(mapping.Value, utils.RegexpConfig{}) + if err != nil { + warnings = append(warnings, fmt.Sprintf("case-sensitive expression %q is not a valid regexp", mapping.Value)) + continue TraitMappingLoop + } + } + // Run the replacement case-sensitively to see if it matches. // If there's no match, the trait specifies a mapping which is case-sensitive; // we should log a warning but return an error. // See https://github.com/gravitational/teleport/issues/6016 for details. - if _, err := utils.ReplaceRegexp(mapping.Value, role, traitValue); err != nil { + if _, err := utils.ReplaceRegexpWith(regexp, role, traitValue); err != nil { warnings = append(warnings, fmt.Sprintf("trait %q matches value %q case-insensitively and would have yielded %q role", traitValue, mapping.Value, outRole)) continue } @@ -108,9 +138,21 @@ func traitsToRoles(ms types.TraitMappingSet, traits map[string][]string, collect } } } + + // show at most maxMismatchedTraitValuesLogged trait values to prevent huge log lines + switch l := len(mismatched); { + case l > maxMismatchedTraitValuesLogged: + log.WithField("expression", mapping.Value). + WithField("values", mismatched[0:maxMismatchedTraitValuesLogged]). + Debugf("%d trait value(s) did not match (showing first %d values)", len(mismatched), maxMismatchedTraitValuesLogged) + case l > 0: + log.WithField("expression", mapping.Value). + WithField("values", mismatched). + Debugf("%d trait value(s) did not match", len(mismatched)) + } } } - return warnings + return } // literalMatcher is used to "escape" values which are not allowed to diff --git a/lib/services/user_test.go b/lib/services/user_test.go index 351436353562c..c03dbe50bb94f 100644 --- a/lib/services/user_test.go +++ b/lib/services/user_test.go @@ -19,23 +19,21 @@ package services import ( "encoding/json" "fmt" + "testing" apidefaults "github.com/gravitational/teleport/api/defaults" + "github.com/gravitational/teleport/api/types" "github.com/coreos/go-oidc/jose" + "github.com/google/go-cmp/cmp" saml2 "github.com/russellhaering/gosaml2" samltypes "github.com/russellhaering/gosaml2/types" - "gopkg.in/check.v1" - - "github.com/gravitational/teleport/api/types" + "github.com/stretchr/testify/require" ) -type UserSuite struct { -} - -var _ = check.Suite(&UserSuite{}) +func TestTraits(t *testing.T) { + t.Parallel() -func (s *UserSuite) TestTraits(c *check.C) { var tests = []struct { traitName string }{ @@ -65,184 +63,216 @@ func (s *UserSuite) TestTraits(c *check.C) { } data, err := json.Marshal(user) - c.Assert(err, check.IsNil) + require.NoError(t, err) _, err = UnmarshalUser(data) - c.Assert(err, check.IsNil) + require.NoError(t, err) } } -func (s *UserSuite) TestOIDCMapping(c *check.C) { - type input struct { - comment string - claims jose.Claims - expectedRoles []string - warnings []string - } - testCases := []struct { - comment string - mappings []types.ClaimMapping - inputs []input - }{ - { - comment: "no mappings", - inputs: []input{ - { - claims: jose.Claims{"a": "b"}, - expectedRoles: nil, - }, +type oidcInput struct { + comment string + claims jose.Claims + expectedRoles []string + warnings []string +} + +var oidcTestCases = []struct { + comment string + mappings []types.ClaimMapping + inputs []oidcInput +}{ + { + comment: "no mappings", + inputs: []oidcInput{ + { + comment: "no match", + claims: jose.Claims{"a": "b"}, + expectedRoles: nil, }, }, - { - comment: "simple mappings", - mappings: []types.ClaimMapping{ - {Claim: "role", Value: "admin", Roles: []string{"admin", "bob"}}, - {Claim: "role", Value: "user", Roles: []string{"user"}}, + }, + { + comment: "simple mappings", + mappings: []types.ClaimMapping{ + {Claim: "role", Value: "admin", Roles: []string{"admin", "bob"}}, + {Claim: "role", Value: "user", Roles: []string{"user"}}, + }, + inputs: []oidcInput{ + { + comment: "no match", + claims: jose.Claims{"a": "b"}, + expectedRoles: nil, }, - inputs: []input{ - { - comment: "no match", - claims: jose.Claims{"a": "b"}, - expectedRoles: nil, - }, - { - comment: "no value match", - claims: jose.Claims{"role": "b"}, - expectedRoles: nil, - }, - { - comment: "direct admin value match", - claims: jose.Claims{"role": "admin"}, - expectedRoles: []string{"admin", "bob"}, - }, - { - comment: "direct user value match", - claims: jose.Claims{"role": "user"}, - expectedRoles: []string{"user"}, - }, - { - comment: "direct user value match with array", - claims: jose.Claims{"role": []string{"user"}}, - expectedRoles: []string{"user"}, - }, + { + comment: "no value match", + claims: jose.Claims{"role": "b"}, + expectedRoles: nil, }, - }, - { - comment: "regexp mappings match", - mappings: []types.ClaimMapping{ - {Claim: "role", Value: "^admin-(.*)$", Roles: []string{"role-$1", "bob"}}, + { + comment: "direct admin value match", + claims: jose.Claims{"role": "admin"}, + expectedRoles: []string{"admin", "bob"}, }, - inputs: []input{ - { - comment: "no match", - claims: jose.Claims{"a": "b"}, - expectedRoles: nil, - }, - { - comment: "no match - subprefix", - claims: jose.Claims{"role": "adminz"}, - expectedRoles: nil, - }, - { - comment: "value with capture match", - claims: jose.Claims{"role": "admin-hello"}, - expectedRoles: []string{"role-hello", "bob"}, - }, - { - comment: "multiple value with capture match, deduplication", - claims: jose.Claims{"role": []string{"admin-hello", "admin-ola"}}, - expectedRoles: []string{"role-hello", "bob", "role-ola"}, - }, - { - comment: "first matches, second does not", - claims: jose.Claims{"role": []string{"hello", "admin-ola"}}, - expectedRoles: []string{"role-ola", "bob"}, - }, + { + comment: "direct user value match", + claims: jose.Claims{"role": "user"}, + expectedRoles: []string{"user"}, + }, + { + comment: "direct user value match with array", + claims: jose.Claims{"role": []string{"user"}}, + expectedRoles: []string{"user"}, }, }, - { - comment: "empty expands are skipped", - mappings: []types.ClaimMapping{ - {Claim: "role", Value: "^admin-(.*)$", Roles: []string{"$2", "bob"}}, + }, + { + comment: "regexp mappings match", + mappings: []types.ClaimMapping{ + {Claim: "role", Value: "^admin-(.*)$", Roles: []string{"role-$1", "bob"}}, + }, + inputs: []oidcInput{ + { + comment: "no match", + claims: jose.Claims{"a": "b"}, + expectedRoles: nil, + }, + { + comment: "no match - subprefix", + claims: jose.Claims{"role": "adminz"}, + expectedRoles: nil, + }, + { + comment: "value with capture match", + claims: jose.Claims{"role": "admin-hello"}, + expectedRoles: []string{"role-hello", "bob"}, }, - inputs: []input{ - { - comment: "value with capture match", - claims: jose.Claims{"role": "admin-hello"}, - expectedRoles: []string{"bob"}, + { + comment: "multiple value with capture match, deduplication", + claims: jose.Claims{"role": []string{"admin-hello", "admin-ola"}}, + expectedRoles: []string{"role-hello", "bob", "role-ola"}, + }, + { + comment: "first matches, second does not", + claims: jose.Claims{"role": []string{"hello", "admin-ola"}}, + expectedRoles: []string{"role-ola", "bob"}, + }, + }, + }, + { + comment: "regexp compilation", + mappings: []types.ClaimMapping{ + {Claim: "role", Value: `^admin-(?!)$`, Roles: []string{"admin"}}, // "?!" is invalid. + {Claim: "role", Value: "^admin-(.*)$", Roles: []string{"role-$1", "bob"}}, + {Claim: "role", Value: `^admin2-(?!)$`, Roles: []string{"admin2"}}, // "?!" is invalid. + }, + inputs: []oidcInput{ + { + comment: "invalid regexp", + claims: jose.Claims{"role": []string{"admin-hello", "dev"}}, + expectedRoles: []string{"role-hello", "bob"}, + warnings: []string{ + `case-insensitive expression "^admin-(?!)$" is not a valid regexp`, + `case-insensitive expression "^admin2-(?!)$" is not a valid regexp`, }, }, + { + comment: "regexp are not compiled if not needed", + claims: jose.Claims{}, + expectedRoles: nil, + // if the regexp were compiled, we would have the same warnings as above + warnings: nil, + }, }, - { - comment: "glob wildcard match", - mappings: []types.ClaimMapping{ - {Claim: "role", Value: "*", Roles: []string{"admin"}}, + }, + { + comment: "empty expands are skipped", + mappings: []types.ClaimMapping{ + {Claim: "role", Value: "^admin-(.*)$", Roles: []string{"$2", "bob"}}, + }, + inputs: []oidcInput{ + { + comment: "value with capture match", + claims: jose.Claims{"role": "admin-hello"}, + expectedRoles: []string{"bob"}, }, - inputs: []input{ - { - comment: "empty value match", - claims: jose.Claims{"role": ""}, - expectedRoles: []string{"admin"}, - }, - { - comment: "any value match", - claims: jose.Claims{"role": "zz"}, - expectedRoles: []string{"admin"}, - }, + }, + }, + { + comment: "glob wildcard match", + mappings: []types.ClaimMapping{ + {Claim: "role", Value: "*", Roles: []string{"admin"}}, + }, + inputs: []oidcInput{ + { + comment: "empty value match", + claims: jose.Claims{"role": ""}, + expectedRoles: []string{"admin"}, + }, + { + comment: "any value match", + claims: jose.Claims{"role": "zz"}, + expectedRoles: []string{"admin"}, }, }, - { - comment: "Whitespace/dashes", - mappings: []types.ClaimMapping{ - {Claim: "groups", Value: "DemoCorp - Backend Engineers", Roles: []string{"backend"}}, - {Claim: "groups", Value: "DemoCorp - SRE Managers", Roles: []string{"approver"}}, - {Claim: "groups", Value: "DemoCorp - SRE", Roles: []string{"approver"}}, - {Claim: "groups", Value: "DemoCorp Infrastructure", Roles: []string{"approver", "backend"}}, + }, + { + comment: "Whitespace/dashes", + mappings: []types.ClaimMapping{ + {Claim: "groups", Value: "DemoCorp - Backend Engineers", Roles: []string{"backend"}}, + {Claim: "groups", Value: "DemoCorp - SRE Managers", Roles: []string{"approver"}}, + {Claim: "groups", Value: "DemoCorp - SRE", Roles: []string{"approver"}}, + {Claim: "groups", Value: "DemoCorp Infrastructure", Roles: []string{"approver", "backend"}}, + }, + inputs: []oidcInput{ + { + comment: "Matches multiple groups", + claims: jose.Claims{ + "groups": []string{"DemoCorp - Backend Engineers", "DemoCorp Infrastructure"}, + }, + expectedRoles: []string{"backend", "approver"}, }, - inputs: []input{ - { - comment: "Matches multiple groups", - claims: jose.Claims{ - "groups": []string{"DemoCorp - Backend Engineers", "DemoCorp Infrastructure"}, - }, - expectedRoles: []string{"backend", "approver"}, + { + comment: "Matches one group", + claims: jose.Claims{ + "groups": []string{"DemoCorp - SRE"}, }, - { - comment: "Matches one group", - claims: jose.Claims{ - "groups": []string{"DemoCorp - SRE"}, - }, - expectedRoles: []string{"approver"}, + expectedRoles: []string{"approver"}, + }, + { + comment: "Matches one group with multiple roles", + claims: jose.Claims{ + "groups": []string{"DemoCorp Infrastructure"}, }, - { - comment: "Matches one group with multiple roles", - claims: jose.Claims{ - "groups": []string{"DemoCorp Infrastructure"}, - }, - expectedRoles: []string{"approver", "backend"}, + expectedRoles: []string{"approver", "backend"}, + }, + { + comment: "No match only due to case-sensitivity", + claims: jose.Claims{ + "groups": []string{"Democorp - SRE"}, }, - { - comment: "No match only due to case-sensitivity", - claims: jose.Claims{ - "groups": []string{"Democorp - SRE"}, - }, - expectedRoles: []string(nil), - warnings: []string{`trait "Democorp - SRE" matches value "DemoCorp - SRE" case-insensitively and would have yielded "approver" role`}, + expectedRoles: []string(nil), + warnings: []string{ + `trait "Democorp - SRE" matches value "DemoCorp - SRE" case-insensitively and would have yielded "approver" role`, }, }, }, - } + }, +} - for i, testCase := range testCases { +func TestOIDCMapping(t *testing.T) { + t.Parallel() + + for i, testCase := range oidcTestCases { conn := types.OIDCConnectorV3{ Spec: types.OIDCConnectorSpecV3{ ClaimsToRoles: testCase.mappings, }, } for _, input := range testCase.inputs { - comment := check.Commentf("OIDC Test case %v %q, input %q", i, testCase.comment, input.comment) + comment := fmt.Sprintf("OIDC Test case %v %q, input %q", i, testCase.comment, input.comment) _, outRoles := TraitsToRoles(conn.GetTraitMappings(), OIDCClaimsToTraits(input.claims)) - c.Assert(outRoles, check.DeepEquals, input.expectedRoles, comment) + require.Empty(t, cmp.Diff(outRoles, input.expectedRoles), comment) } samlConn := types.SAMLConnectorV2{ @@ -251,10 +281,30 @@ func (s *UserSuite) TestOIDCMapping(c *check.C) { }, } for _, input := range testCase.inputs { - comment := check.Commentf("SAML Test case %v %v, input %#v", i, testCase.comment, input) + comment := fmt.Sprintf("SAML Test case %v %v, input %#v", i, testCase.comment, input) warnings, outRoles := TraitsToRoles(samlConn.GetTraitMappings(), SAMLAssertionsToTraits(claimsToAttributes(input.claims))) - c.Assert(outRoles, check.DeepEquals, input.expectedRoles, comment) - c.Assert(warnings, check.DeepEquals, input.warnings, comment) + require.Empty(t, cmp.Diff(outRoles, input.expectedRoles), comment) + require.Empty(t, cmp.Diff(warnings, input.warnings), comment) + } + } +} +func BenchmarkTraitToRoles(b *testing.B) { + for _, testCase := range oidcTestCases { + samlConn := types.SAMLConnectorV2{ + Spec: types.SAMLConnectorSpecV2{ + AttributesToRoles: claimMappingsToAttributeMappings(testCase.mappings), + }, + } + mappings := samlConn.GetTraitMappings() + for _, input := range testCase.inputs { + testCaseInputName := fmt.Sprintf("%s %s", testCase.comment, input.comment) + traits := SAMLAssertionsToTraits(claimsToAttributes(input.claims)) + + b.Run(testCaseInputName, func(b *testing.B) { + for i := 0; i < b.N; i++ { + TraitsToRoles(mappings, traits) + } + }) } } } diff --git a/lib/services/usertoken_test.go b/lib/services/usertoken_test.go index aad8ab6783048..079dd66082a8c 100644 --- a/lib/services/usertoken_test.go +++ b/lib/services/usertoken_test.go @@ -13,25 +13,26 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ + package services import ( + "fmt" + "testing" "time" - "gopkg.in/check.v1" - "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/fixtures" -) -type UserTokenSuite struct{} + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" +) -var _ = check.Suite(&UserTokenSuite{}) +func TestUserTokenUnmarshal(t *testing.T) { + t.Parallel() -func (r *UserTokenSuite) TestUnmarshal(c *check.C) { created, err := time.Parse(time.RFC3339, "2020-01-14T18:52:39.523076855Z") - c.Assert(err, check.IsNil) + require.NoError(t, err) type testCase struct { description string @@ -73,14 +74,14 @@ func (r *UserTokenSuite) TestUnmarshal(c *check.C) { } for _, tc := range testCases { - comment := check.Commentf("test case %q", tc.description) + comment := fmt.Sprintf("test case %q", tc.description) out, err := UnmarshalUserToken([]byte(tc.input)) - c.Assert(err, check.IsNil, comment) - fixtures.DeepCompare(c, tc.expected, out) + require.NoError(t, err, comment) + require.Empty(t, cmp.Diff(tc.expected, out)) data, err := MarshalUserToken(out) - c.Assert(err, check.IsNil, comment) + require.NoError(t, err, comment) out2, err := UnmarshalUserToken(data) - c.Assert(err, check.IsNil, comment) - fixtures.DeepCompare(c, tc.expected, out2) + require.NoError(t, err, comment) + require.Empty(t, cmp.Diff(tc.expected, out2)) } } diff --git a/lib/services/wrappers_test.go b/lib/services/wrappers_test.go index 33d6f2bef7f8e..453e310640d30 100644 --- a/lib/services/wrappers_test.go +++ b/lib/services/wrappers_test.go @@ -20,31 +20,26 @@ import ( "encoding/hex" "testing" - "gopkg.in/check.v1" - "github.com/gravitational/teleport/api/types/wrappers" -) - -type WrappersSuite struct{} -var _ = check.Suite(&WrappersSuite{}) - -func TestWrappers(t *testing.T) { check.TestingT(t) } + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" +) -func (w *WrappersSuite) TestUnmarshalBackwards(c *check.C) { +func TestUnmarshalBackwards(t *testing.T) { var traits wrappers.Traits // Attempt to unmarshal protobuf encoded data. protoBytes := "0a120a066c6f67696e7312080a06666f6f6261720a150a116b756265726e657465735f67726f7570731200" data, err := hex.DecodeString(protoBytes) - c.Assert(err, check.IsNil) + require.NoError(t, err) err = wrappers.UnmarshalTraits(data, &traits) - c.Assert(err, check.IsNil) - c.Assert(traits["logins"], check.DeepEquals, []string{"foobar"}) + require.NoError(t, err) + require.Empty(t, cmp.Diff(traits["logins"], []string{"foobar"})) // Attempt to unmarshal JSON encoded data. jsonBytes := `{"logins": ["foobar"]}` err = wrappers.UnmarshalTraits([]byte(jsonBytes), &traits) - c.Assert(err, check.IsNil) - c.Assert(traits["logins"], check.DeepEquals, []string{"foobar"}) + require.NoError(t, err) + require.Empty(t, cmp.Diff(traits["logins"], []string{"foobar"})) } diff --git a/lib/utils/replace.go b/lib/utils/replace.go index 4d22501f29eef..47d6ac004645d 100644 --- a/lib/utils/replace.go +++ b/lib/utils/replace.go @@ -24,7 +24,7 @@ import ( // ContainsExpansion returns true if value contains // expansion syntax, e.g. $1 or ${10} func ContainsExpansion(val string) bool { - return reExpansion.FindAllStringIndex(val, -1) != nil + return reExpansion.FindStringIndex(val) != nil } // GlobToRegexp replaces glob-style standalone wildcard values @@ -35,19 +35,23 @@ func GlobToRegexp(in string) string { } // ReplaceRegexp replaces value in string, accepts regular expression and simplified -// wildcard syntax, it has several important differeneces with standard lib +// wildcard syntax, it has several important differences with standard lib // regexp replacer: // * Wildcard globs '*' are treated as regular expression .* expression // * Expression is treated as regular expression if it starts with ^ and ends with $ // * Full match is expected, partial replacements ignored // * If there is no match, returns a NotFound error func ReplaceRegexp(expression string, replaceWith string, input string) (string, error) { - return ReplaceRegexpWithConfig(expression, replaceWith, input, RegexpConfig{}) + expr, err := RegexpWithConfig(expression, RegexpConfig{}) + if err != nil { + return "", trace.Wrap(err) + } + return ReplaceRegexpWith(expr, replaceWith, input) } -// ReplaceRegexpWithConfig behaves exactly like ReplaceRegexp but its behavior -// can be customized -func ReplaceRegexpWithConfig(expression string, replaceWith string, input string, config RegexpConfig) (string, error) { +// RegexpWithConfig compiles a regular expression given some configuration. +// There are several important differences with standard lib (see ReplaceRegexp). +func RegexpWithConfig(expression string, config RegexpConfig) (*regexp.Regexp, error) { if !strings.HasPrefix(expression, "^") || !strings.HasSuffix(expression, "$") { // replace glob-style wildcards with regexp wildcards // for plain strings, and quote all characters that could @@ -59,11 +63,16 @@ func ReplaceRegexpWithConfig(expression string, replaceWith string, input string } expr, err := regexp.Compile(expression) if err != nil { - return "", trace.BadParameter(err.Error()) + return nil, trace.BadParameter(err.Error()) } + return expr, nil +} + +// ReplaceRegexp replaces string in a given regexp. +func ReplaceRegexpWith(expr *regexp.Regexp, replaceWith string, input string) (string, error) { // if there is no match, return NotFound error - index := expr.FindAllStringIndex(input, -1) - if len(index) == 0 { + index := expr.FindStringIndex(input) + if index == nil { return "", trace.NotFound("no match found") } return expr.ReplaceAllString(input, replaceWith), nil