diff --git a/api/go.mod b/api/go.mod index 63809dcf536..c66aade7faf 100644 --- a/api/go.mod +++ b/api/go.mod @@ -5,7 +5,7 @@ go 1.15 require ( github.com/gogo/protobuf v1.3.1 github.com/golang/protobuf v1.4.2 - github.com/google/go-cmp v0.4.0 + github.com/google/go-cmp v0.5.4 github.com/gravitational/trace v1.1.15 github.com/jonboulle/clockwork v0.2.2 // indirect github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect diff --git a/api/go.sum b/api/go.sum index 91da7e68722..15c417232ef 100644 --- a/api/go.sum +++ b/api/go.sum @@ -23,8 +23,9 @@ github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= +github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/gravitational/trace v1.1.13 h1:4vSuBtVQCbUGj28dm5ke8UPnuejcxeSlz9GEW9X7HIM= github.com/gravitational/trace v1.1.13/go.mod h1:RvdOUHE4SHqR3oXlFFKnGzms8a5dugHygGw1bqDstYI= github.com/gravitational/trace v1.1.15 h1:dfaFcARt110nCX6RSvrcRUbvRawEYAasXyCqnhXo0Xg= diff --git a/api/types/access_request.go b/api/types/access_request.go index 20a6a0bee2c..b52689a10e6 100644 --- a/api/types/access_request.go +++ b/api/types/access_request.go @@ -93,8 +93,6 @@ type AccessRequest interface { // CheckAndSetDefaults validates the access request and // supplies default values where appropriate. CheckAndSetDefaults() error - // Equals checks equality between access request values. - Equals(AccessRequest) bool } // NewAccessRequest assembled an AccessRequest resource. @@ -383,30 +381,10 @@ func (r *AccessRequestV3) String() string { return fmt.Sprintf("AccessRequest(user=%v,roles=%+v)", r.Spec.User, r.Spec.Roles) } -// Equals compares two AccessRequests -func (r *AccessRequestV3) Equals(other AccessRequest) bool { - o, ok := other.(*AccessRequestV3) - if !ok { - return false - } - if r.GetName() != o.GetName() { - return false - } - return r.Spec.Equals(&o.Spec) -} - -func (t AccessReviewThreshold) Equals(other AccessReviewThreshold) bool { - return reflect.DeepEqual(t, other) -} - func (c AccessReviewConditions) IsZero() bool { return reflect.ValueOf(c).IsZero() } -func (c AccessRequestConditions) IsZero() bool { - return reflect.ValueOf(c).IsZero() -} - func (s AccessReviewSubmission) Check() error { if s.RequestID == "" { return trace.BadParameter("missing request ID") @@ -544,28 +522,6 @@ func (s RequestState) IsResolved() bool { return s.IsApproved() || s.IsDenied() } -// Equals compares two AccessRequestSpecs -func (s *AccessRequestSpecV3) Equals(other *AccessRequestSpecV3) bool { - if s.User != other.User { - return false - } - if len(s.Roles) != len(other.Roles) { - return false - } - for i, role := range s.Roles { - if role != other.Roles[i] { - return false - } - } - if s.Created != other.Created { - return false - } - if s.Expires != other.Expires { - return false - } - return s.State == other.State -} - // key values for map encoding of request filter const ( keyID = "id" @@ -620,8 +576,3 @@ func (f *AccessRequestFilter) Match(req AccessRequest) bool { } return true } - -// Equals compares two AccessRequestFilters -func (f *AccessRequestFilter) Equals(o AccessRequestFilter) bool { - return f.ID == o.ID && f.User == o.User && f.State == o.State -} diff --git a/api/types/plugin_data.go b/api/types/plugin_data.go index ca8aa415fdf..39f25d32839 100644 --- a/api/types/plugin_data.go +++ b/api/types/plugin_data.go @@ -236,19 +236,3 @@ func (f *PluginDataFilter) Match(data PluginData) bool { } return true } - -// Equals compares two PluginDataEntries -func (d *PluginDataEntry) Equals(other *PluginDataEntry) bool { - if other == nil { - return false - } - if len(d.Data) != len(other.Data) { - return false - } - for key, val := range d.Data { - if other.Data[key] != val { - return false - } - } - return true -} diff --git a/api/types/role.go b/api/types/role.go index 7f0dfcad59b..20615febea8 100644 --- a/api/types/role.go +++ b/api/types/role.go @@ -36,9 +36,6 @@ type Role interface { Resource // CheckAndSetDefaults checks and set default values for any missing fields. CheckAndSetDefaults() error - // Equals returns true if the roles are equal. Roles are equal if options and - // conditions match. - Equals(other Role) bool // GetOptions gets role options. GetOptions() RoleOptions @@ -151,52 +148,6 @@ const ( Deny RoleConditionType = false ) -// Equals returns true if the roles are equal. Roles are equal if options, -// namespaces, logins, labels, and conditions match. -func (r *RoleV3) Equals(other Role) bool { - if !r.GetOptions().Equals(other.GetOptions()) { - return false - } - - for _, condition := range []RoleConditionType{Allow, Deny} { - if !utils.StringSlicesEqual(r.GetLogins(condition), other.GetLogins(condition)) { - return false - } - if !utils.StringSlicesEqual(r.GetNamespaces(condition), other.GetNamespaces(condition)) { - return false - } - if !r.GetNodeLabels(condition).Equals(other.GetNodeLabels(condition)) { - return false - } - if !r.GetAppLabels(condition).Equals(other.GetAppLabels(condition)) { - return false - } - if !r.GetDatabaseLabels(condition).Equals(other.GetDatabaseLabels(condition)) { - return false - } - if !utils.StringSlicesEqual(r.GetDatabaseNames(condition), other.GetDatabaseNames(condition)) { - return false - } - if !utils.StringSlicesEqual(r.GetDatabaseUsers(condition), other.GetDatabaseUsers(condition)) { - return false - } - if !RuleSlicesEqual(r.GetRules(condition), other.GetRules(condition)) { - return false - } - if !r.GetClusterLabels(condition).Equals(other.GetClusterLabels(condition)) { - return false - } - if !r.GetKubernetesLabels(condition).Equals(other.GetKubernetesLabels(condition)) { - return false - } - if !r.GetImpersonateConditions(condition).Equals(other.GetImpersonateConditions(condition)) { - return false - } - } - - return true -} - // GetVersion returns resource version func (r *RoleV3) GetVersion() string { return r.Version @@ -669,75 +620,11 @@ func (r *RoleV3) String() string { r.GetName(), r.Spec.Options, r.Spec.Allow, r.Spec.Deny) } -// Equals checks if all the key/values in the RoleOptions map match. -func (o RoleOptions) Equals(other RoleOptions) bool { - return (o.ForwardAgent.Value() == other.ForwardAgent.Value() && - o.MaxSessionTTL.Value() == other.MaxSessionTTL.Value() && - BoolDefaultTrue(o.PortForwarding) == BoolDefaultTrue(other.PortForwarding) && - o.CertificateFormat == other.CertificateFormat && - o.ClientIdleTimeout.Value() == other.ClientIdleTimeout.Value() && - o.DisconnectExpiredCert.Value() == other.DisconnectExpiredCert.Value() && - utils.StringSlicesEqual(o.BPF, other.BPF)) -} - -// Equals returns true if the role conditions (logins, namespaces, labels, -// and rules) are equal and false if they are not. -func (r *RoleConditions) Equals(o RoleConditions) bool { - if !utils.StringSlicesEqual(r.Logins, o.Logins) { - return false - } - if !utils.StringSlicesEqual(r.Namespaces, o.Namespaces) { - return false - } - if !r.NodeLabels.Equals(o.NodeLabels) { - return false - } - if !r.AppLabels.Equals(o.AppLabels) { - return false - } - if !r.KubernetesLabels.Equals(o.KubernetesLabels) { - return false - } - if !r.DatabaseLabels.Equals(o.DatabaseLabels) { - return false - } - if !utils.StringSlicesEqual(r.DatabaseNames, o.DatabaseNames) { - return false - } - if !utils.StringSlicesEqual(r.DatabaseUsers, o.DatabaseUsers) { - return false - } - if len(r.Rules) != len(o.Rules) { - return false - } - for i := range r.Rules { - if !r.Rules[i].Equals(o.Rules[i]) { - return false - } - } - return true -} - // IsEmpty returns true if conditions are unspecified func (i ImpersonateConditions) IsEmpty() bool { return len(i.Users) == 0 || len(i.Roles) == 0 } -// Equals returns true if the impersonate conditions (logins, roles -// and rules) are equal and false if they are not. -func (i ImpersonateConditions) Equals(o ImpersonateConditions) bool { - if !utils.StringSlicesEqual(i.Users, o.Users) { - return false - } - if !utils.StringSlicesEqual(i.Roles, o.Roles) { - return false - } - if i.Where != o.Where { - return false - } - return true -} - // CheckAndSetDefaults checks and sets default values func (i ImpersonateConditions) CheckAndSetDefaults() error { if len(i.Users) != 0 && len(i.Roles) == 0 { @@ -795,23 +682,6 @@ func (r *Rule) HasVerb(verb string) bool { return false } -// Equals returns true if the rule equals to another -func (r *Rule) Equals(other Rule) bool { - if !utils.StringSlicesEqual(r.Resources, other.Resources) { - return false - } - if !utils.StringSlicesEqual(r.Verbs, other.Verbs) { - return false - } - if !utils.StringSlicesEqual(r.Actions, other.Actions) { - return false - } - if r.Where != other.Where { - return false - } - return true -} - // CopyRulesSlice copies input slice of Rules and returns the copy func CopyRulesSlice(in []Rule) []Rule { out := make([]Rule, len(in)) @@ -819,19 +689,6 @@ func CopyRulesSlice(in []Rule) []Rule { return out } -// RuleSlicesEqual returns true if two rule slices are equal -func RuleSlicesEqual(a, b []Rule) bool { - if len(a) != len(b) { - return false - } - for i := range a { - if !a[i].Equals(b[i]) { - return false - } - } - return true -} - // Labels is a wrapper around map // that can marshal and unmarshal itself // from scalar and list values @@ -897,19 +754,6 @@ func (l Labels) Clone() Labels { return out } -// Equals returns true if two label sets are equal -func (l Labels) Equals(o Labels) bool { - if len(l) != len(o) { - return false - } - for key := range l { - if !utils.StringSlicesEqual(l[key], o[key]) { - return false - } - } - return true -} - // NewBool returns Bool struct based on bool value func NewBool(b bool) Bool { return Bool(b) diff --git a/api/types/saml.go b/api/types/saml.go index c06100e2fe8..ed5dbaf0d00 100644 --- a/api/types/saml.go +++ b/api/types/saml.go @@ -53,8 +53,6 @@ type SAMLConnector interface { GetSigningKeyPair() *AsymmetricKeyPair // GetSigningKeyPair sets signing key pair SetSigningKeyPair(k *AsymmetricKeyPair) - // Equals returns true if the connectors are identical - Equals(other SAMLConnector) bool // GetSSO returns SSO service GetSSO() string // SetSSO sets SSO service @@ -219,49 +217,6 @@ func (o *SAMLConnectorV2) SetAssertionConsumerService(v string) { o.Spec.AssertionConsumerService = v } -// Equals returns true if the connectors are identical -func (o *SAMLConnectorV2) Equals(other SAMLConnector) bool { - if o.GetName() != other.GetName() { - return false - } - if o.GetCert() != other.GetCert() { - return false - } - if o.GetAudience() != other.GetAudience() { - return false - } - if o.GetEntityDescriptor() != other.GetEntityDescriptor() { - return false - } - if o.Expiry() != other.Expiry() { - return false - } - if o.GetIssuer() != other.GetIssuer() { - return false - } - if (o.GetSigningKeyPair() == nil && other.GetSigningKeyPair() != nil) || (o.GetSigningKeyPair() != nil && other.GetSigningKeyPair() == nil) { - return false - } - if o.GetSigningKeyPair() != nil { - a, b := o.GetSigningKeyPair(), other.GetSigningKeyPair() - if a.Cert != b.Cert || a.PrivateKey != b.PrivateKey { - return false - } - } - mappings := o.GetAttributesToRoles() - otherMappings := other.GetAttributesToRoles() - if len(mappings) != len(otherMappings) { - return false - } - for i := range mappings { - a, b := mappings[i], otherMappings[i] - if a.Name != b.Name || a.Value != b.Value || !utils.StringSlicesEqual(a.Roles, b.Roles) { - return false - } - } - return o.GetSSO() == other.GetSSO() -} - // SetDisplay sets friendly name for this provider. func (o *SAMLConnectorV2) SetDisplay(display string) { o.Spec.Display = display diff --git a/api/types/server.go b/api/types/server.go index bb987d23668..00938a7a0ab 100644 --- a/api/types/server.go +++ b/api/types/server.go @@ -23,11 +23,8 @@ import ( "strings" "time" - "github.com/gravitational/teleport/api/utils" - - "github.com/gravitational/trace" - "github.com/gogo/protobuf/proto" + "github.com/gravitational/trace" ) // Server represents a Node, Proxy or Auth server in a Teleport cluster @@ -354,23 +351,6 @@ type CommandLabel interface { GetCommand() []string // Clone returns label copy Clone() CommandLabel - // Equals returns true if label is equal to the other one - // false otherwise - Equals(CommandLabel) bool -} - -// Equals returns true if labels are equal, false otherwise -func (c *CommandLabelV2) Equals(other CommandLabel) bool { - if c.GetPeriod() != other.GetPeriod() { - return false - } - if c.GetResult() != other.GetResult() { - return false - } - if !utils.StringSlicesEqual(c.GetCommand(), other.GetCommand()) { - return false - } - return true } // Clone returns non-shallow copy of the label diff --git a/api/types/session.go b/api/types/session.go index 5155e3612b4..15398ca1719 100644 --- a/api/types/session.go +++ b/api/types/session.go @@ -581,8 +581,3 @@ func (f *WebSessionFilter) Match(session WebSession) bool { } return true } - -// Equals compares two filters. -func (f *WebSessionFilter) Equals(o WebSessionFilter) bool { - return f.User == o.User -} diff --git a/api/types/trustedcluster.go b/api/types/trustedcluster.go index 721235068fd..0439600047e 100644 --- a/api/types/trustedcluster.go +++ b/api/types/trustedcluster.go @@ -20,6 +20,8 @@ import ( "fmt" "time" + "github.com/google/go-cmp/cmp" + "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/utils" @@ -251,7 +253,7 @@ func (c *TrustedClusterV2) CanChangeStateTo(t TrustedCluster) error { if !utils.StringSlicesEqual(c.GetRoles(), t.GetRoles()) { return immutableFieldErr("roles") } - if !c.GetRoleMap().Equals(t.GetRoleMap()) { + if !cmp.Equal(c.GetRoleMap(), t.GetRoleMap()) { return immutableFieldErr("role_map") } @@ -271,33 +273,9 @@ func (c *TrustedClusterV2) String() string { c.Spec.Enabled, c.Spec.Roles, c.Spec.Token, c.Spec.ProxyAddress, c.Spec.ReverseTunnelAddress) } -// Equals checks if the two role mappings are equal. -func (r RoleMapping) Equals(o RoleMapping) bool { - if r.Remote != o.Remote { - return false - } - if !utils.StringSlicesEqual(r.Local, r.Local) { - return false - } - return true -} - // RoleMap is a list of mappings type RoleMap []RoleMapping -// Equals checks if the two role maps are equal. -func (r RoleMap) Equals(o RoleMap) bool { - if len(r) != len(o) { - return false - } - for i := range r { - if !r[i].Equals(o[i]) { - return false - } - } - return true -} - // SortedTrustedCluster sorts clusters by name type SortedTrustedCluster []TrustedCluster diff --git a/api/types/user.go b/api/types/user.go index c4f29506377..4448416c589 100644 --- a/api/types/user.go +++ b/api/types/user.go @@ -308,11 +308,6 @@ func (i *ExternalIdentity) String() string { return fmt.Sprintf("OIDCIdentity(connectorID=%v, username=%v)", i.ConnectorID, i.Username) } -// Equals returns true if this identity equals to passed one -func (i *ExternalIdentity) Equals(other *ExternalIdentity) bool { - return i.ConnectorID == other.ConnectorID && i.Username == other.Username -} - // Check returns nil if all parameters are great, err otherwise func (i *ExternalIdentity) Check() error { if i.ConnectorID == "" { diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index eb56e035697..c06b7fb0176 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -29,6 +29,7 @@ import ( "golang.org/x/crypto/ssh" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/constants" @@ -887,12 +888,12 @@ func (s *AuthSuite) TestUpsertDeleteRoleEventsEmitted(c *C) { roleRetrieved, err := s.a.GetRole(ctx, "test") c.Assert(err, IsNil) - c.Assert(roleRetrieved.Equals(roleTest), Equals, true) + c.Assert(cmp.Diff(roleRetrieved, roleTest, cmpopts.IgnoreFields(types.Metadata{}, "ID")), Equals, "") // test update role err = s.a.upsertRole(ctx, roleTest) c.Assert(err, IsNil) - c.Assert(roleRetrieved.Equals(roleTest), Equals, true) + c.Assert(cmp.Diff(roleRetrieved, roleTest, cmpopts.IgnoreFields(types.Metadata{}, "ID")), Equals, "") c.Assert(s.mockEmitter.LastEvent().GetType(), DeepEquals, events.RoleCreatedEvent) c.Assert(s.mockEmitter.LastEvent().(*events.RoleCreate).Name, Equals, "test") s.mockEmitter.Reset() diff --git a/lib/auth/saml.go b/lib/auth/saml.go index b459608eab1..9a3f07f59e8 100644 --- a/lib/auth/saml.go +++ b/lib/auth/saml.go @@ -24,6 +24,8 @@ import ( "fmt" "io/ioutil" + "github.com/google/go-cmp/cmp" + "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/defaults" @@ -123,7 +125,7 @@ func (a *Server) getSAMLProvider(conn services.SAMLConnector) (*saml2.SAMLServic defer a.lock.Unlock() providerPack, ok := a.samlProviders[conn.GetName()] - if ok && providerPack.connector.Equals(conn) { + if ok && cmp.Equal(providerPack.connector, conn) { return providerPack.provider, nil } delete(a.samlProviders, conn.GetName()) diff --git a/lib/service/service.go b/lib/service/service.go index d9cb743845a..024e0c78789 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -2292,7 +2292,7 @@ func (process *TeleportProcess) setupProxyListeners() (*proxyListeners, error) { case cfg.Proxy.DisableWebService && cfg.Proxy.DisableReverseTunnel: process.log.Debugf("Setup Proxy: Reverse tunnel proxy and web proxy are disabled.") return &listeners, nil - case cfg.Proxy.ReverseTunnelListenAddr.Equals(cfg.Proxy.WebAddr) && !cfg.Proxy.DisableTLS: + case cfg.Proxy.ReverseTunnelListenAddr == cfg.Proxy.WebAddr && !cfg.Proxy.DisableTLS: process.log.Debugf("Setup Proxy: Reverse tunnel proxy and web proxy listen on the same port, multiplexing is on.") listener, err := process.importOrCreateListener(listenerProxyTunnelAndWeb, cfg.Proxy.WebAddr.Addr) if err != nil { diff --git a/lib/services/access_request.go b/lib/services/access_request.go index f05d391edb6..c65b0ea663d 100644 --- a/lib/services/access_request.go +++ b/lib/services/access_request.go @@ -21,6 +21,8 @@ import ( "fmt" "sort" + "github.com/google/go-cmp/cmp" + "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/parse" @@ -1049,7 +1051,7 @@ func (c *thresholdCollector) pushThreshold(t types.AccessReviewThreshold) (uint3 // don't bother double-storing equivalent thresholds for i, threshold := range c.Thresholds { - if t.Equals(threshold) { + if cmp.Equal(t, threshold) { return uint32(i), nil } } diff --git a/lib/services/access_request_test.go b/lib/services/access_request_test.go index c4391f63a4d..e3607198e52 100644 --- a/lib/services/access_request_test.go +++ b/lib/services/access_request_test.go @@ -22,10 +22,9 @@ import ( "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/gravitational/trace" - "github.com/pborman/uuid" "github.com/stretchr/testify/require" ) @@ -630,7 +629,7 @@ func TestAccessRequestMarshaling(t *testing.T) { req2, err := UnmarshalAccessRequest(marshaled) require.NoError(t, err) - require.True(t, req1.Equals(req2)) + require.True(t, cmp.Equal(req1, req2)) } // TestPluginDataExpectations verifies the correct behavior of the `Expect` mapping. @@ -810,17 +809,12 @@ func TestRequestFilterConversion(t *testing.T) { }, } for _, tc := range testCases { - - if m := tc.f.IntoMap(); !utils.StringMapsEqual(m, tc.m) { - t.Errorf("bad map encoding: expected %+v, got %+v", tc.m, m) - } + m := tc.f.IntoMap() + require.Empty(t, cmp.Diff(m, tc.m)) var f AccessRequestFilter - if err := f.FromMap(tc.m); err != nil { - t.Errorf("failed to parse %+v: %s", tc.m, err) - } - if !f.Equals(tc.f) { - t.Errorf("bad map decoding: expected %+v, got %+v", tc.f, f) - } + err := f.FromMap(tc.m) + require.NoError(t, err) + require.Empty(t, cmp.Diff(f, tc.f)) } badMaps := []map[string]string{ {"food": "carrots"}, diff --git a/lib/services/authentication.go b/lib/services/authentication.go index 8cb6631622b..a205eadbd3d 100644 --- a/lib/services/authentication.go +++ b/lib/services/authentication.go @@ -19,7 +19,6 @@ limitations under the License. package services import ( - "bytes" "encoding/json" "fmt" "time" @@ -53,37 +52,6 @@ func ValidateLocalAuthSecrets(l *LocalAuthSecrets) error { return nil } -// LocalAuthSecretsEquals checks equality (nil safe). -func LocalAuthSecretsEquals(l *LocalAuthSecrets, other *LocalAuthSecrets) bool { - if (l == nil) || (other == nil) { - return l == other - } - if !bytes.Equal(l.PasswordHash, other.PasswordHash) { - return false - } - if len(l.MFA) != len(other.MFA) { - return false - } - mfa := make(map[string]*types.MFADevice, len(l.MFA)) - for i, d := range l.MFA { - mfa[d.Id] = l.MFA[i] - } - mfaOther := make(map[string]*types.MFADevice, len(other.MFA)) - for i, d := range other.MFA { - mfaOther[d.Id] = other.MFA[i] - } - for id, d := range mfa { - od, ok := mfaOther[id] - if !ok { - return false - } - if !mfaDeviceEquals(d, od) { - return false - } - } - return true -} - // NewTOTPDevice creates a TOTP MFADevice from the given key. func NewTOTPDevice(name, key string, addedAt time.Time) (*types.MFADevice, error) { d := types.NewMFADevice(name, uuid.New(), addedAt) @@ -126,59 +94,6 @@ func validateTOTPDevice(d *types.TOTPDevice) error { return nil } -func mfaDeviceEquals(d, other *types.MFADevice) bool { - if (d == nil) || (other == nil) { - return d == other - } - if d.Kind != other.Kind { - return false - } - if d.SubKind != other.SubKind { - return false - } - if d.Version != other.Version { - return false - } - if d.Metadata.Name != other.Metadata.Name { - return false - } - if d.Id != other.Id { - return false - } - if !d.AddedAt.Equal(other.AddedAt) { - return false - } - // Ignore LastUsed, it's a very dynamic field. - if !totpDeviceEquals(d.GetTotp(), other.GetTotp()) { - return false - } - if !u2fDeviceEquals(d.GetU2F(), other.GetU2F()) { - return false - } - return true -} - -func totpDeviceEquals(d, other *types.TOTPDevice) bool { - if (d == nil) || (other == nil) { - return d == other - } - return d.Key == other.Key -} - -func u2fDeviceEquals(d, other *types.U2FDevice) bool { - if (d == nil) || (other == nil) { - return d == other - } - if !bytes.Equal(d.KeyHandle, other.KeyHandle) { - return false - } - if !bytes.Equal(d.PubKey, other.PubKey) { - return false - } - // Ignore the counter, it's a very dynamic value. - return true -} - // AuthPreferenceSpecSchemaTemplate is JSON schema for AuthPreferenceSpec const AuthPreferenceSpecSchemaTemplate = `{ "type": "object", diff --git a/lib/services/local/users.go b/lib/services/local/users.go index 4c7a67adf5a..9965e3c22ba 100644 --- a/lib/services/local/users.go +++ b/lib/services/local/users.go @@ -25,6 +25,8 @@ import ( "golang.org/x/crypto/bcrypt" + "github.com/google/go-cmp/cmp" + "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth/u2f" "github.com/gravitational/teleport/lib/backend" @@ -273,7 +275,7 @@ func (s *IdentityService) GetUserByOIDCIdentity(id services.ExternalIdentity) (s } for _, u := range users { for _, uid := range u.GetOIDCIdentities() { - if uid.Equals(&id) { + if cmp.Equal(uid, &id) { return u, nil } } @@ -290,7 +292,7 @@ func (s *IdentityService) GetUserBySAMLIdentity(id services.ExternalIdentity) (s } for _, u := range users { for _, uid := range u.GetSAMLIdentities() { - if uid.Equals(&id) { + if cmp.Equal(uid, &id) { return u, nil } } @@ -306,7 +308,7 @@ func (s *IdentityService) GetUserByGithubIdentity(id services.ExternalIdentity) } for _, u := range users { for _, uid := range u.GetGithubIdentities() { - if uid.Equals(&id) { + if cmp.Equal(uid, &id) { return u, nil } } diff --git a/lib/services/role_test.go b/lib/services/role_test.go index 5aedf7190d1..5607219fac1 100644 --- a/lib/services/role_test.go +++ b/lib/services/role_test.go @@ -24,6 +24,8 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/wrappers" @@ -440,7 +442,7 @@ func TestRoleParse(t *testing.T) { } } else { require.NoError(t, err) - require.True(t, role.Equals(&tc.role)) + require.True(t, cmp.Equal(role, &tc.role)) err := ValidateRole(role) require.NoError(t, err) @@ -450,7 +452,7 @@ func TestRoleParse(t *testing.T) { role2, err := UnmarshalRole(out) require.NoError(t, err) - require.True(t, role2.Equals(&tc.role)) + require.True(t, cmp.Equal(role2, &tc.role)) } }) } diff --git a/lib/services/server.go b/lib/services/server.go index ba26d5f018e..c965b5b0d2f 100644 --- a/lib/services/server.go +++ b/lib/services/server.go @@ -39,24 +39,22 @@ const ( Different = iota ) -// Compare compares two provided resources. -func Compare(a, b Resource) int { +// CompareServers compares two provided servers. +func CompareServers(a, b Resource) int { if serverA, ok := a.(Server); ok { if serverB, ok := b.(Server); ok { - return CompareServers(serverA, serverB) + return compareServers(serverA, serverB) } } if dbA, ok := a.(types.DatabaseServer); ok { if dbB, ok := b.(types.DatabaseServer); ok { - return CompareDatabaseServers(dbA, dbB) + return compareDatabaseServers(dbA, dbB) } } return Different } -// CompareServers returns difference between two server -// objects, Equal (0) if identical, OnlyTimestampsDifferent(1) if only timestamps differ, Different(2) otherwise -func CompareServers(a, b Server) int { +func compareServers(a, b Server) int { if a.GetKind() != b.GetKind() { return Different } @@ -85,7 +83,7 @@ func CompareServers(a, b Server) int { if !utils.StringMapsEqual(a.GetLabels(), b.GetLabels()) { return Different } - if !CmdLabelMapsEqual(a.GetCmdLabels(), b.GetCmdLabels()) { + if !cmp.Equal(a.GetCmdLabels(), b.GetCmdLabels()) { return Different } if !a.Expiry().Equal(b.Expiry()) { @@ -95,58 +93,16 @@ func CompareServers(a, b Server) int { return Different } - // If this server is proxying applications, compare them to make sure they match. - if a.GetKind() == KindAppServer { - return CompareApps(a.GetApps(), b.GetApps()) + if !cmp.Equal(a.GetApps(), b.GetApps()) { + return Different } - if !cmp.Equal(a.GetKubernetesClusters(), b.GetKubernetesClusters()) { return Different } - return Equal } -// CompareApps compares two slices of apps and returns if they are equal or -// different. -func CompareApps(a []*App, b []*App) int { - if len(a) != len(b) { - return Different - } - for i := range a { - if a[i].Name != b[i].Name { - return Different - } - if a[i].URI != b[i].URI { - return Different - } - if a[i].PublicAddr != b[i].PublicAddr { - return Different - } - if !utils.StringMapsEqual(a[i].StaticLabels, b[i].StaticLabels) { - return Different - } - if !CmdLabelMapsEqual( - V2ToLabels(a[i].DynamicLabels), - V2ToLabels(b[i].DynamicLabels)) { - return Different - } - if (a[i].Rewrite == nil && b[i].Rewrite != nil) || - (a[i].Rewrite != nil && b[i].Rewrite == nil) { - return Different - } - if a[i].Rewrite != nil && b[i].Rewrite != nil { - if !utils.StringSlicesEqual(a[i].Rewrite.Redirect, b[i].Rewrite.Redirect) { - return Different - } - } - } - return Equal -} - -// CompareDatabaseServers returns whether the two provided database servers -// are equal or different. -func CompareDatabaseServers(a, b types.DatabaseServer) int { +func compareDatabaseServers(a, b types.DatabaseServer) int { if a.GetKind() != b.GetKind() { return Different } @@ -166,7 +122,7 @@ func CompareDatabaseServers(a, b types.DatabaseServer) int { if !utils.StringMapsEqual(a.GetStaticLabels(), b.GetStaticLabels()) { return Different } - if !CmdLabelMapsEqual(a.GetDynamicLabels(), b.GetDynamicLabels()) { + if !cmp.Equal(a.GetDynamicLabels(), b.GetDynamicLabels()) { return Different } if !a.Expiry().Equal(b.Expiry()) { @@ -181,24 +137,6 @@ func CompareDatabaseServers(a, b types.DatabaseServer) int { return Equal } -// CmdLabelMapsEqual compares two maps with command labels, -// returns true if label sets are equal -func CmdLabelMapsEqual(a, b map[string]CommandLabel) bool { - if len(a) != len(b) { - return false - } - for key, val := range a { - val2, ok := b[key] - if !ok { - return false - } - if !val.Equals(val2) { - return false - } - } - return true -} - // CommandLabels is a set of command labels type CommandLabels map[string]CommandLabel diff --git a/lib/services/types.go b/lib/services/types.go index 9202db3fb2e..f9809dc4a17 100644 --- a/lib/services/types.go +++ b/lib/services/types.go @@ -318,7 +318,6 @@ var ( NewRole = types.NewRole NewRule = types.NewRule CopyRulesSlice = types.CopyRulesSlice - RuleSlicesEqual = types.RuleSlicesEqual NewBool = types.NewBool NewBoolOption = types.NewBoolOption BoolDefaultTrue = types.BoolDefaultTrue diff --git a/lib/services/user.go b/lib/services/user.go index 3dfc193b71a..e0ab9aaa029 100644 --- a/lib/services/user.go +++ b/lib/services/user.go @@ -21,8 +21,12 @@ import ( "fmt" "time" - "github.com/gravitational/teleport/lib/utils" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/gravitational/trace" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/utils" ) // ValidateUser validates the User and sets default values @@ -40,37 +44,12 @@ func ValidateUser(u User) error { // UsersEquals checks if the users are equal func UsersEquals(u User, other User) bool { - if u.GetName() != other.GetName() { - return false - } - otherIdentities := other.GetOIDCIdentities() - if len(u.GetOIDCIdentities()) != len(otherIdentities) { - return false - } - for i := range u.GetOIDCIdentities() { - if !u.GetOIDCIdentities()[i].Equals(&otherIdentities[i]) { - return false - } - } - otherSAMLIdentities := other.GetSAMLIdentities() - if len(u.GetSAMLIdentities()) != len(otherSAMLIdentities) { - return false - } - for i := range u.GetSAMLIdentities() { - if !u.GetSAMLIdentities()[i].Equals(&otherSAMLIdentities[i]) { - return false - } - } - otherGithubIdentities := other.GetGithubIdentities() - if len(u.GetGithubIdentities()) != len(otherGithubIdentities) { - return false - } - for i := range u.GetGithubIdentities() { - if !u.GetGithubIdentities()[i].Equals(&otherGithubIdentities[i]) { - return false - } - } - return LocalAuthSecretsEquals(u.GetLocalAuth(), other.GetLocalAuth()) + return cmp.Equal(u, other, + cmpopts.IgnoreFields(types.Metadata{}, "ID"), + cmpopts.SortSlices(func(a, b *types.MFADevice) bool { + return a.Metadata.Name < b.Metadata.Name + }), + ) } // LoginAttempt represents successful or unsuccessful attempt for user to login diff --git a/lib/srv/heartbeat.go b/lib/srv/heartbeat.go index 442020b1de4..e1031dc2c8b 100644 --- a/lib/srv/heartbeat.go +++ b/lib/srv/heartbeat.go @@ -334,7 +334,7 @@ func (h *Heartbeat) fetch() error { h.reset(HeartbeatStateAnnounce) return nil } - result := services.Compare(h.current, server) + result := services.CompareServers(h.current, server) // server update happened, time to announce if result == services.Different { h.current = server @@ -352,7 +352,7 @@ func (h *Heartbeat) fetch() error { h.setState(HeartbeatStateKeepAlive) return nil } - result := services.Compare(h.current, server) + result := services.CompareServers(h.current, server) // server update happened, move to announce if result == services.Different { h.current = server diff --git a/lib/utils/addr.go b/lib/utils/addr.go index 9fffc80aee1..c25f93e873a 100644 --- a/lib/utils/addr.go +++ b/lib/utils/addr.go @@ -65,11 +65,6 @@ func (a *NetAddr) Port(defaultPort int) int { return porti } -// Equals returns true if address is equal to other -func (a *NetAddr) Equals(other NetAddr) bool { - return a.Addr == other.Addr && a.AddrNetwork == other.AddrNetwork && a.Path == other.Path -} - // IsLocal returns true if this is a local address func (a *NetAddr) IsLocal() bool { host, _, err := net.SplitHostPort(a.Addr) diff --git a/lib/utils/loadbalancer.go b/lib/utils/loadbalancer.go index acd523d4235..441eb47f9ac 100644 --- a/lib/utils/loadbalancer.go +++ b/lib/utils/loadbalancer.go @@ -116,7 +116,7 @@ func (l *LoadBalancer) RemoveBackend(b NetAddr) error { defer l.Unlock() l.currentIndex = -1 for i := range l.backends { - if l.backends[i].Equals(b) { + if l.backends[i] == b { l.backends = append(l.backends[:i], l.backends[i+1:]...) l.dropConnections(b) return nil diff --git a/vendor/modules.txt b/vendor/modules.txt index 13751d081c1..e1c5c03ee4d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -277,8 +277,11 @@ github.com/gravitational/roundtrip ## explicit github.com/gravitational/teleport/api/client github.com/gravitational/teleport/api/client/proto +github.com/gravitational/teleport/api/client/webclient github.com/gravitational/teleport/api/constants github.com/gravitational/teleport/api/defaults +github.com/gravitational/teleport/api/identityfile +github.com/gravitational/teleport/api/profile github.com/gravitational/teleport/api/types github.com/gravitational/teleport/api/types/events github.com/gravitational/teleport/api/types/wrappers