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.

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

add memberClusters selection #87

Merged
merged 2 commits into from
Oct 18, 2024
Merged

add memberClusters selection #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09db381
add memberClusters selection
MatousJobanek Oct 18, 2024
5a3540c
test names
MatousJobanek Oct 18, 2024
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
17 changes: 17 additions & 0 deletions pkg/assets/kubesaw-admins.go → pkg/assets/kubesaw_admins.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package assets

import "k8s.io/utils/strings/slices"

type KubeSawAdmins struct {
Clusters Clusters `yaml:"clusters"`
ServiceAccounts []ServiceAccount `yaml:"serviceAccounts"`
Expand Down Expand Up @@ -43,6 +45,21 @@ type ServiceAccount struct {
type Selector struct {
// SkipMembers can contain a list of member cluster names the entity shouldn't be applied for
SkipMembers []string `yaml:"skipMembers,omitempty"`
// MemberClusters defines a list of member cluster names the entity should be applied for
MemberClusters []string `yaml:"memberClusters,omitempty"`
}

func (s Selector) ShouldBeSkippedForMember(memberName string) bool {
// should be skipped if the specific member cluster name is provided
// and
// the name is listed in the skipped members
if memberName != "" && slices.Contains(s.SkipMembers, memberName) {
return true
}
// should be skipped if there is at least one selected member cluster
// and
// the name is either empty or is not specified in the selected member clusters
return len(s.MemberClusters) > 0 && (memberName == "" || !slices.Contains(s.MemberClusters, memberName))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that there can be no "apply this entity to any member cluster"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. It's already the default behavior for the cases when you don't specify any permissions in any member cluster.

}

type User struct {
Expand Down
66 changes: 66 additions & 0 deletions pkg/assets/kubesaw_admins_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package assets

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestShouldBeSkippedForMember1(t *testing.T) {
// given
member1Member2 := []string{"member1", "member2"}
member2Member3 := []string{"member2", "member3"}
testCases := map[string]struct {
s Selector
shouldBeSkipped bool
mfrancisc marked this conversation as resolved.
Show resolved Hide resolved
}{
"no selector": {s: Selector{}, shouldBeSkipped: false},
"different selected members": {s: Selector{MemberClusters: member2Member3}, shouldBeSkipped: true},
"in selected members": {s: Selector{MemberClusters: member1Member2}, shouldBeSkipped: false},
"listed in skipped members": {s: Selector{SkipMembers: member1Member2}, shouldBeSkipped: true},
"not listed in skipped members": {s: Selector{SkipMembers: member2Member3}, shouldBeSkipped: false},
"in selected members, but listed in skipped": {
s: Selector{MemberClusters: member1Member2, SkipMembers: member1Member2}, shouldBeSkipped: true},
"in selected members, not listed in skipped": {
s: Selector{MemberClusters: member1Member2, SkipMembers: member2Member3}, shouldBeSkipped: false},
"different selected members, not listed in skipped": {
s: Selector{MemberClusters: member2Member3, SkipMembers: member2Member3}, shouldBeSkipped: true},
"different selected members, and listed in skipped": {
s: Selector{MemberClusters: member2Member3, SkipMembers: member1Member2}, shouldBeSkipped: true},
}

for testName, data := range testCases {
t.Run(testName, func(t *testing.T) {
// when
shouldBeSkipped := data.s.ShouldBeSkippedForMember("member1")

// then
assert.Equal(t, data.shouldBeSkipped, shouldBeSkipped)
})
}
}

func TestShouldBeSkippedForEmptyName(t *testing.T) {
// given
member1Member2 := []string{"member1", "member2"}
testCases := map[string]struct {
s Selector
shouldBeSkipped bool
}{
"no selector": {s: Selector{}, shouldBeSkipped: false},
"some selected members": {s: Selector{MemberClusters: member1Member2}, shouldBeSkipped: true},
"some skipped members": {s: Selector{SkipMembers: member1Member2}, shouldBeSkipped: false},
"some selected members and some skipped members": {
s: Selector{MemberClusters: member1Member2, SkipMembers: member1Member2}, shouldBeSkipped: true},
}

for testName, data := range testCases {
t.Run(testName, func(t *testing.T) {
// when
shouldBeSkipped := data.s.ShouldBeSkippedForMember("")
mfrancisc marked this conversation as resolved.
Show resolved Hide resolved

// then
assert.Equal(t, data.shouldBeSkipped, shouldBeSkipped)
})
}
}
23 changes: 21 additions & 2 deletions pkg/cmd/generate/admin-manifests_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,21 @@ func TestAdminManifests(t *testing.T) {
WithSkippedMembers("member2"),
Sa("bob", "",
HostRoleBindings("toolchain-host-operator", Role("restart-deployment"), ClusterRole("edit")),
MemberRoleBindings("toolchain-member-operator", Role("restart-deployment"), ClusterRole("edit")))),
MemberRoleBindings("toolchain-member-operator", Role("restart-deployment"), ClusterRole("edit"))),
Sa("jenny", "",
MemberRoleBindings("toolchain-member-operator", Role("restart-deployment"), ClusterRole("view"))).
WithSelectedMembers("member2")),
Users(
User("john-user", []string{"12345"}, false, "crtadmins-view",
HostRoleBindings("toolchain-host-operator", Role("register-cluster"), ClusterRole("edit")),
MemberRoleBindings("toolchain-member-operator", Role("register-cluster"), ClusterRole("edit"))).
WithSkippedMembers("member2"),
User("bob-crtadmin", []string{"67890"}, false, "crtadmins-exec",
HostRoleBindings("toolchain-host-operator", Role("restart-deployment"), ClusterRole("admin")),
MemberRoleBindings("toolchain-member-operator", Role("restart-deployment"), ClusterRole("admin")))))
MemberRoleBindings("toolchain-member-operator", Role("restart-deployment"), ClusterRole("admin"))),
User("jenny-crtadmin", []string{"98765"}, false, "crtadmins-exec",
MemberRoleBindings("toolchain-member-operator", Role("restart-deployment"), ClusterRole("view"))).
WithSelectedMembers("member2")))
kubeSawAdmins.DefaultServiceAccountsNamespace.Host = "kubesaw-sre-host"
kubeSawAdminsContent, err := yaml.Marshal(kubeSawAdmins)
require.NoError(t, err)
Expand Down Expand Up @@ -274,6 +280,11 @@ func verifyServiceAccounts(t *testing.T, outDir, expectedRootDir string, cluster
assertSa(saNs, "john").
hasRole(roleNs, clusterType.AsSuffix("install-operator"), clusterType.AsSuffix("install-operator-john")).
hasNsClusterRole(roleNs, "admin", clusterType.AsSuffix("clusterrole-admin-john"))
} else {
inKStructure(t, outDir, expectedRootDir).
assertSa(saNs, "jenny").
hasRole(roleNs, clusterType.AsSuffix("restart-deployment"), clusterType.AsSuffix("restart-deployment-jenny")).
hasNsClusterRole(roleNs, "view", clusterType.AsSuffix("clusterrole-view-jenny"))
}
inKStructure(t, outDir, expectedRootDir).
assertSa(saNs, "bob").
Expand Down Expand Up @@ -302,6 +313,14 @@ func verifyUsers(t *testing.T, outDir, expectedRootDir string, clusterType confi

// crtadmins-view group is not generated for member2 at all
bobsExtraGroupsUserIsNotPartOf = extraGroupsUserIsNotPartOf("crtadmins-view")
} else {
inKStructure(t, outDir, rootDir).
assertUser("jenny-crtadmin").
hasIdentity("98765")

newPermissionAssertion(storageAssertion, "", "jenny-crtadmin", "User").
hasRole(ns, clusterType.AsSuffix("restart-deployment"), clusterType.AsSuffix("restart-deployment-jenny-crtadmin")).
hasNsClusterRole(ns, "view", clusterType.AsSuffix("clusterrole-view-jenny-crtadmin"))
}

inKStructure(t, outDir, rootDir).
Expand Down
3 changes: 1 addition & 2 deletions pkg/cmd/generate/cli_configs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"k8s.io/client-go/tools/clientcmd"
"k8s.io/kubectl/pkg/scheme"
"k8s.io/utils/pointer"
"k8s.io/utils/strings/slices"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
)

Expand Down Expand Up @@ -185,7 +184,7 @@ func generateForCluster(ctx *generateContext, clusterType configuration.ClusterT
tokenPerSAName := tokenPerSA{}

for _, sa := range ctx.kubeSawAdmins.ServiceAccounts {
if slices.Contains(sa.Selector.SkipMembers, clusterName) {
if clusterType == configuration.Member && sa.Selector.ShouldBeSkippedForMember(clusterName) {
continue
}
for saClusterType := range sa.PermissionsPerClusterType {
Expand Down
13 changes: 11 additions & 2 deletions pkg/cmd/generate/cli_configs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ func TestGenerateCliConfigs(t *testing.T) {
HostRoleBindings("toolchain-host-operator", Role("install-operator"), ClusterRole("admin")),
MemberRoleBindings("toolchain-member-operator", Role("install-operator"), ClusterRole("admin"))).
WithSkippedMembers("member2"),
Sa("jenny", "",
HostRoleBindings("toolchain-host-operator", Role("restart-deployment"), ClusterRole("view")),
MemberRoleBindings("toolchain-member-operator", Role("restart-deployment"), ClusterRole("view"))).
WithSelectedMembers("member2"),
Sa("bob", "",
HostRoleBindings("toolchain-host-operator", Role("restart=restart-deployment"), ClusterRole("restart=edit")),
MemberRoleBindings("toolchain-member-operator", Role("restart=restart-deployment"), ClusterRole("restart=edit")))),
Expand All @@ -54,13 +58,15 @@ func TestGenerateCliConfigs(t *testing.T) {
setupGockForServiceAccounts(t, HostServerAPI, 50,
newServiceAccount("kubesaw-sre-host", "john"),
newServiceAccount("kubesaw-sre-host", "bob"),
newServiceAccount("kubesaw-sre-host", "jenny"),
)
setupGockForServiceAccounts(t, Member1ServerAPI, 50,
newServiceAccount("kubesaw-admins-member", "john"),
newServiceAccount("kubesaw-admins-member", "bob"),
)
setupGockForServiceAccounts(t, Member2ServerAPI, 50,
newServiceAccount("kubesaw-admins-member", "bob"),
newServiceAccount("kubesaw-admins-member", "jenny"),
)
t.Cleanup(gock.OffAll)

Expand All @@ -87,7 +93,8 @@ func TestGenerateCliConfigs(t *testing.T) {

verifyKsctlConfigFiles(t, tempDir,
cliConfigForUser("john", hasHost(), hasMember("member1", "member1")),
cliConfigForUser("bob", hasHost(), hasMember("member1", "member1"), hasMember("member2", "member2")))
cliConfigForUser("bob", hasHost(), hasMember("member1", "member1"), hasMember("member2", "member2")),
cliConfigForUser("jenny", hasHost(), hasMember("member2", "member2")))
})

t.Run("when there SAs are defined for host cluster only", func(t *testing.T) {
Expand Down Expand Up @@ -127,6 +134,7 @@ func TestGenerateCliConfigs(t *testing.T) {
setupGockForServiceAccounts(t, HostServerAPI, 50,
newServiceAccount("kubesaw-admins-member", "john"),
newServiceAccount("kubesaw-admins-member", "bob"),
newServiceAccount("kubesaw-admins-member", "jenny"),
)
tempDir, err := os.MkdirTemp("", "ksctl-out-")
require.NoError(t, err)
Expand All @@ -141,7 +149,8 @@ func TestGenerateCliConfigs(t *testing.T) {

verifyKsctlConfigFiles(t, tempDir,
cliConfigForUser("john", hasHost(), hasMember("member1", "host")),
cliConfigForUser("bob", hasHost(), hasMember("member1", "host"), hasMember("member2", "host")))
cliConfigForUser("bob", hasHost(), hasMember("member1", "host"), hasMember("member2", "host")),
cliConfigForUser("jenny", hasHost(), hasMember("member2", "host")))
})
})

Expand Down
5 changes: 2 additions & 3 deletions pkg/cmd/generate/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package generate

import (
"github.com/kubesaw/ksctl/pkg/configuration"
"k8s.io/utils/strings/slices"
)

type clusterContext struct {
Expand All @@ -16,7 +15,7 @@ type clusterContext struct {
func ensureServiceAccounts(ctx *clusterContext, objsCache objectsCache) error {
ctx.Printlnf("-> Ensuring ServiceAccounts and its RoleBindings...")
for _, sa := range ctx.kubeSawAdmins.ServiceAccounts {
if ctx.specificKMemberName != "" && slices.Contains(sa.Selector.SkipMembers, ctx.specificKMemberName) {
if sa.Selector.ShouldBeSkippedForMember(ctx.specificKMemberName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we keep the empty name check ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's already part of ShouldBeSkippedForMember method logic

continue
}

Expand Down Expand Up @@ -47,7 +46,7 @@ func ensureUsers(ctx *clusterContext, objsCache objectsCache) error {
ctx.Printlnf("-> Ensuring Users and its RoleBindings...")

for _, user := range ctx.kubeSawAdmins.Users {
if ctx.specificKMemberName != "" && slices.Contains(user.Selector.SkipMembers, ctx.specificKMemberName) {
if user.Selector.ShouldBeSkippedForMember(ctx.specificKMemberName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we keep the empty name check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same, the check is part of the method

continue
}
m := &permissionsManager{
Expand Down
16 changes: 16 additions & 0 deletions pkg/test/environment_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ func (c ServiceAccountCreator) WithSkippedMembers(members ...string) ServiceAcco
}
}

func (c ServiceAccountCreator) WithSelectedMembers(members ...string) ServiceAccountCreator {
return func() assets.ServiceAccount {
serviceAccount := c()
serviceAccount.Selector.MemberClusters = members
return serviceAccount
}
}

func NewPermissionsPerClusterType(permissions ...PermissionsPerClusterTypeModifier) assets.PermissionsPerClusterType {
perm := map[string]assets.PermissionBindings{}
for _, addPermissions := range permissions {
Expand Down Expand Up @@ -179,3 +187,11 @@ func (c UserCreator) WithSkippedMembers(members ...string) UserCreator {
return user
}
}

func (c UserCreator) WithSelectedMembers(members ...string) UserCreator {
return func() assets.User {
user := c()
user.Selector.MemberClusters = members
return user
}
}
Loading