Use cmp.Equal instead of manual Equals methods (#5828)

* Use cmp.Equal instead of manual Equals methods

Equals methods can get out-of-sync with the fields added in structs they
compare. Using `cmp.Equal` handles that, removes a ton of code and makes
it more explicit when specific fields are excluded from comparison.

* Use gogoproto equal plugin for comparing proto values

This will be faster than reflect-based go-cmp.

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

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=

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
-}

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
-}

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)

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

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

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
-}

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
 

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 == "" {

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()

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())

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 {

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
 		}
 	}

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"},

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",

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
 			}
 		}

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))
 			}
 		})
 	}

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
 

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

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

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

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)

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

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