Skip to content
New issue

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

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

Do not add username labels to Identities and Users #92

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion pkg/cmd/generate/assertion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ type userAssertion struct {
func (a *storageAssertionImpl) assertUser(name string) userAssertion {
expLabels := map[string]string{
"provider": "ksctl",
"username": name,
}

userObj := &userv1.User{}
Expand Down
9 changes: 5 additions & 4 deletions pkg/cmd/generate/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ func ensureServiceAccounts(ctx *clusterContext, objsCache objectsCache) error {
}

pm := &permissionsManager{
objectsCache: objsCache,
createSubject: ensureServiceAccount(saNamespace),
subjectBaseName: sa.Name,
objectsCache: objsCache,
createSubject: ensureServiceAccount(saNamespace),
subjectBaseName: sa.Name,
objectIsServiceAccount: true,
}

if err := pm.ensurePermissions(ctx, sa.PermissionsPerClusterType); err != nil {
Expand Down Expand Up @@ -56,7 +57,7 @@ func ensureUsers(ctx *clusterContext, objsCache objectsCache) error {
}
// create the subject if explicitly requested (even if there is no specific permissions)
if user.AllClusters {
if _, err := m.createSubject(ctx, m.objectsCache, m.subjectBaseName, defaultSAsNamespace(ctx.kubeSawAdmins, ctx.clusterType), ksctlLabelsWithUsername(m.subjectBaseName)); err != nil {
if _, err := m.createSubject(ctx, m.objectsCache, m.subjectBaseName, defaultSAsNamespace(ctx.kubeSawAdmins, ctx.clusterType), ksctlLabels()); err != nil {
return err
}
}
Expand Down
14 changes: 11 additions & 3 deletions pkg/cmd/generate/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ import (

type permissionsManager struct {
objectsCache
createSubject newSubjectFunc
subjectBaseName string
createSubject newSubjectFunc
subjectBaseName string
objectIsServiceAccount bool
}

type newSubjectFunc func(ctx *clusterContext, objsCache objectsCache, subjectBaseName, targetNamespace string, labels map[string]string) (rbacv1.Subject, error)
Expand Down Expand Up @@ -81,7 +82,14 @@ func (m *permissionsManager) ensurePermission(ctx *clusterContext, roleName, tar
}

// ensure that the subject exists
subject, err := m.createSubject(ctx, m.objectsCache, m.subjectBaseName, defaultSAsNamespace(ctx.kubeSawAdmins, ctx.clusterType), ksctlLabelsWithUsername(m.subjectBaseName))
labels := ksctlLabels()
if m.objectIsServiceAccount {
// It might be useful to have the corresponding username in labels in the SA
// However we don't want to add the username label to Identities and Users because usernames are not DNS compliant and not always can be used as labels
// TODO don't use the raw username as a label in SA too. We could use annotations instead.
labels = ksctlLabelsWithUsername(m.subjectBaseName)
}
subject, err := m.createSubject(ctx, m.objectsCache, m.subjectBaseName, defaultSAsNamespace(ctx.kubeSawAdmins, ctx.clusterType), labels)
if err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/cmd/generate/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ func TestEnsureServiceAccount(t *testing.T) {
func TestEnsureUserAndIdentity(t *testing.T) {
labels := map[string]string{
"provider": "ksctl",
"username": "john-crtadmin",
}
require.NoError(t, client.AddToScheme())

Expand Down Expand Up @@ -222,8 +221,9 @@ func newPermissionsManager(t *testing.T, clusterType configuration.ClusterType,
cache := objectsCache{}

return permissionsManager{
objectsCache: cache,
subjectBaseName: "john",
createSubject: ensureServiceAccount(""),
objectsCache: cache,
subjectBaseName: "john",
createSubject: ensureServiceAccount(""),
objectIsServiceAccount: true,
}, clusterCtx
}
Loading