Skip to content

Propagate resource revision to/from the backend #32040

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rosstimothy merged 3 commits into from
Sep 26, 2023
Merged

Propagate resource revision to/from the backend #32040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25853ca
Set revision on resources retrieved from the backend
rosstimothy Sep 20, 2023
40ccc63
Prevent storing resource revision in the backend item value
rosstimothy Sep 20, 2023
542ea2c
Ignore revision in resource comparisons
rosstimothy Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/auth/apiserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func TestUpsertServer(t *testing.T) {
addServers(s.GetAuthServers())
addServers(s.GetNodes(ctx, apidefaults.Namespace))
addServers(s.GetProxies())
require.Empty(t, cmp.Diff(allServers, []types.Server{tt.wantServer}, cmpopts.IgnoreFields(types.Metadata{}, "ID")))
require.Empty(t, cmp.Diff(allServers, []types.Server{tt.wantServer}, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))
})
}
}
Expand Down
34 changes: 19 additions & 15 deletions lib/auth/auth_with_roles_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"github.com/pquerna/otp/totp"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"golang.org/x/exp/slices"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api"
Expand Down Expand Up @@ -1782,33 +1783,33 @@ func TestDatabasesCRUDRBAC(t *testing.T) {
db, err := devClt.GetDatabase(ctx, devDatabase.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(devDatabase, db,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Admin can get both databases.
db, err = adminClt.GetDatabase(ctx, adminDatabase.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(adminDatabase, db,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))
db, err = adminClt.GetDatabase(ctx, devDatabase.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(devDatabase, db,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// When listing databases, dev should only see one.
dbs, err := devClt.GetDatabases(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Database{devDatabase}, dbs,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Admin should see both.
dbs, err = adminClt.GetDatabases(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Database{adminDatabase, devDatabase}, dbs,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Dev shouldn't be able to delete dev database...
Expand Down Expand Up @@ -1902,7 +1903,7 @@ func mustGetDatabases(t *testing.T, client *Client, wantDatabases []types.Databa
require.NoError(t, err)

require.Empty(t, cmp.Diff(wantDatabases, actualDatabases,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
cmpopts.EquateEmpty(),
))
}
Expand Down Expand Up @@ -1957,7 +1958,7 @@ func TestKubernetesClusterCRUD_DiscoveryService(t *testing.T) {
t.Run("Read", func(t *testing.T) {
clusters, err := discoveryClt.GetKubernetesClusters(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.KubeCluster{eksCluster}, clusters))
require.Empty(t, cmp.Diff([]types.KubeCluster{eksCluster}, clusters, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))
})
t.Run("Update", func(t *testing.T) {
require.NoError(t, discoveryClt.UpdateKubernetesCluster(ctx, eksCluster))
Expand Down Expand Up @@ -2480,33 +2481,33 @@ func TestApps(t *testing.T) {
app, err := devClt.GetApp(ctx, devApp.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(devApp, app,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Admin can get both apps.
app, err = adminClt.GetApp(ctx, adminApp.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(adminApp, app,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))
app, err = adminClt.GetApp(ctx, devApp.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(devApp, app,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// When listing apps, dev should only see one.
apps, err := devClt.GetApps(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Application{devApp}, apps,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Admin should see both.
apps, err = adminClt.GetApps(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Application{adminApp, devApp}, apps,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Dev shouldn't be able to delete dev app...
Expand All @@ -2531,7 +2532,7 @@ func TestApps(t *testing.T) {
apps, err = adminClt.GetApps(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Application{adminApp}, apps,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Admin should be able to delete all.
Expand Down Expand Up @@ -3609,7 +3610,10 @@ func TestListResources_KindUserGroup(t *testing.T) {

userGroups, err := types.ResourcesWithLabels(res.Resources).AsUserGroups()
require.NoError(t, err)
require.ElementsMatch(t, []types.UserGroup{testUg1, testUg2, testUg3}, userGroups)
slices.SortFunc(userGroups, func(a, b types.UserGroup) int {
return strings.Compare(a.GetName(), b.GetName())
})
require.Empty(t, cmp.Diff([]types.UserGroup{testUg2, testUg3, testUg1}, userGroups, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))
})

t.Run("start keys", func(t *testing.T) {
Expand Down Expand Up @@ -6378,7 +6382,7 @@ func TestCreateAccessRequest(t *testing.T) {
// We have to ignore the name here, as it's auto-generated by the underlying access request
// logic.
require.Empty(t, cmp.Diff(test.expected, accessRequests[0],
cmpopts.IgnoreFields(types.Metadata{}, "Name", "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "Name", "ID", "Revision"),
cmpopts.IgnoreFields(types.AccessRequestSpecV3{}),
))
})
Expand Down
54 changes: 27 additions & 27 deletions lib/auth/grpcserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2454,7 +2454,7 @@ func TestNodesCRUD(t *testing.T) {
require.NoError(t, err)
require.Len(t, nodes, 2)
require.Empty(t, cmp.Diff([]types.Server{node1, node2}, nodes,
cmpopts.IgnoreFields(types.Metadata{}, "ID")))
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))

// GetNodes should not fail if namespace is empty
_, err = clt.GetNodes(ctx, "")
Expand All @@ -2466,7 +2466,7 @@ func TestNodesCRUD(t *testing.T) {
node, err := clt.GetNode(ctx, apidefaults.Namespace, "node1")
require.NoError(t, err)
require.Empty(t, cmp.Diff(node1, node,
cmpopts.IgnoreFields(types.Metadata{}, "ID")))
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))

// GetNode should fail if node name isn't provided
_, err = clt.GetNode(ctx, apidefaults.Namespace, "")
Expand Down Expand Up @@ -2565,7 +2565,7 @@ func TestLocksCRUD(t *testing.T) {
require.NoError(t, err)
require.Len(t, locks, 2)
require.Empty(t, cmp.Diff([]types.Lock{lock1, lock2}, locks,
cmpopts.IgnoreFields(types.Metadata{}, "ID")))
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))
})
t.Run("GetLocks with targets", func(t *testing.T) {
t.Parallel()
Expand All @@ -2574,15 +2574,15 @@ func TestLocksCRUD(t *testing.T) {
require.NoError(t, err)
require.Len(t, locks, 2)
require.Empty(t, cmp.Diff([]types.Lock{lock1, lock2}, locks,
cmpopts.IgnoreFields(types.Metadata{}, "ID")))
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))

// Match only one of the locks.
roleTarget := types.LockTarget{Role: "role-A"}
locks, err = clt.GetLocks(ctx, false, lock1.Target(), roleTarget)
require.NoError(t, err)
require.Len(t, locks, 1)
require.Empty(t, cmp.Diff([]types.Lock{lock1}, locks,
cmpopts.IgnoreFields(types.Metadata{}, "ID")))
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))

// Match none of the locks.
locks, err = clt.GetLocks(ctx, false, roleTarget)
Expand All @@ -2595,7 +2595,7 @@ func TestLocksCRUD(t *testing.T) {
lock, err := clt.GetLock(ctx, lock1.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(lock1, lock,
cmpopts.IgnoreFields(types.Metadata{}, "ID")))
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))

// Attempt to get a nonexistent lock.
_, err = clt.GetLock(ctx, "lock3")
Expand Down Expand Up @@ -2675,7 +2675,7 @@ func TestApplicationServersCRUD(t *testing.T) {
out, err = clt.GetApplicationServers(ctx, apidefaults.Namespace)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.AppServer{server1, server2, server3}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Update an app server.
Expand All @@ -2685,7 +2685,7 @@ func TestApplicationServersCRUD(t *testing.T) {
out, err = clt.GetApplicationServers(ctx, apidefaults.Namespace)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.AppServer{server1, server2, server3}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Delete an app server.
Expand All @@ -2694,7 +2694,7 @@ func TestApplicationServersCRUD(t *testing.T) {
out, err = clt.GetApplicationServers(ctx, apidefaults.Namespace)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.AppServer{server2, server3}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Delete all app servers.
Expand Down Expand Up @@ -2745,14 +2745,14 @@ func TestAppsCRUD(t *testing.T) {
out, err = clt.GetApps(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Application{app1, app2}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Fetch a specific app.
app, err := clt.GetApp(ctx, app2.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(app2, app,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Try to fetch an app that doesn't exist.
Expand All @@ -2770,7 +2770,7 @@ func TestAppsCRUD(t *testing.T) {
app, err = clt.GetApp(ctx, app1.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(app1, app,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Delete an app.
Expand All @@ -2779,7 +2779,7 @@ func TestAppsCRUD(t *testing.T) {
out, err = clt.GetApps(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Application{app2}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Try to delete an app that doesn't exist.
Expand Down Expand Up @@ -2826,7 +2826,7 @@ func TestAppServersCRUD(t *testing.T) {

appServer := resources.Resources[0].(types.AppServer)
require.Empty(t, cmp.Diff(appServer, appServer1,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

require.NoError(t, clt.DeleteApplicationServer(ctx, apidefaults.Namespace, "hostID", appServer1.GetName()))
Expand Down Expand Up @@ -2879,7 +2879,7 @@ func TestAppServersCRUD(t *testing.T) {
app2.SetOrigin(types.OriginOkta)
appServer = resources.Resources[0].(types.AppServer)
require.Empty(t, cmp.Diff(appServer, appServer2,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

require.NoError(t, clt.DeleteApplicationServer(ctx, apidefaults.Namespace, "hostID", appServer2.GetName()))
Expand Down Expand Up @@ -2934,14 +2934,14 @@ func TestDatabasesCRUD(t *testing.T) {
out, err = clt.GetDatabases(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Database{db1, db2}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Fetch a specific database.
db, err := clt.GetDatabase(ctx, db2.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(db2, db,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Try to fetch a database that doesn't exist.
Expand All @@ -2959,7 +2959,7 @@ func TestDatabasesCRUD(t *testing.T) {
db, err = clt.GetDatabase(ctx, db1.GetName())
require.NoError(t, err)
require.Empty(t, cmp.Diff(db1, db,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Delete a database.
Expand All @@ -2968,7 +2968,7 @@ func TestDatabasesCRUD(t *testing.T) {
out, err = clt.GetDatabases(ctx)
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.Database{db2}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Try to delete a database that doesn't exist.
Expand Down Expand Up @@ -3050,7 +3050,7 @@ func TestDatabaseServicesCRUD(t *testing.T) {
out, err = types.ResourcesWithLabels(listServicesResp.Resources).AsDatabaseServices()
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.DatabaseService{db1, db2}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Update a DatabaseService.
Expand All @@ -3072,7 +3072,7 @@ func TestDatabaseServicesCRUD(t *testing.T) {
out, err = types.ResourcesWithLabels(listServicesResp.Resources).AsDatabaseServices()
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.DatabaseService{db1, db2}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Delete a DatabaseService.
Expand All @@ -3088,7 +3088,7 @@ func TestDatabaseServicesCRUD(t *testing.T) {
out, err = types.ResourcesWithLabels(listServicesResp.Resources).AsDatabaseServices()
require.NoError(t, err)
require.Empty(t, cmp.Diff([]types.DatabaseService{db2}, out,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Try to delete a DatabaseService that doesn't exist.
Expand Down Expand Up @@ -3151,7 +3151,7 @@ func TestServerInfoCRUD(t *testing.T) {
}

requireResourcesEqual := func(t *testing.T, expected, actual interface{}) {
require.Empty(t, cmp.Diff(expected, actual, cmpopts.IgnoreFields(types.Metadata{}, "ID")))
require.Empty(t, cmp.Diff(expected, actual, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))
}

t.Run("ServerInfoGetters", func(t *testing.T) {
Expand Down Expand Up @@ -3253,7 +3253,7 @@ func TestSAMLIdPServiceProvidersCRUD(t *testing.T) {
require.NoError(t, err)
require.Empty(t, nextKey)
require.Empty(t, cmp.Diff([]types.SAMLIdPServiceProvider{sp1, sp2}, listResp,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Update a service provider.
Expand All @@ -3266,7 +3266,7 @@ func TestSAMLIdPServiceProvidersCRUD(t *testing.T) {
require.NoError(t, err)
require.Empty(t, nextKey)
require.Empty(t, cmp.Diff([]types.SAMLIdPServiceProvider{sp1, sp2}, listResp,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Delete a service provider.
Expand All @@ -3276,7 +3276,7 @@ func TestSAMLIdPServiceProvidersCRUD(t *testing.T) {
require.NoError(t, err)
require.Empty(t, nextKey)
require.Empty(t, cmp.Diff([]types.SAMLIdPServiceProvider{sp2}, listResp,
cmpopts.IgnoreFields(types.Metadata{}, "ID"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision"),
))

// Try to delete a service provider that doesn't exist.
Expand Down Expand Up @@ -4174,7 +4174,7 @@ func TestRoleVersions(t *testing.T) {
return
}
require.Empty(t, cmp.Diff(tc.expectedRole, gotRole,
cmpopts.IgnoreFields(types.RoleV6{}, "Metadata.ID", "Metadata.Labels")))
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision", "Labels")))
// The downgraded label value won't match exactly because it
// includes the client version, so just check it's not empty
// and ignore it in the role diff.
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1339,7 +1339,7 @@ func resourceFromYAML(t *testing.T, value string) types.Resource {

func resourceDiff(res1, res2 types.Resource) string {
return cmp.Diff(res1, res2,
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Namespace"),
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision", "Namespace"),
cmpopts.EquateEmpty())
}

Expand Down
Loading