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

Add nsaccess package to determine if a user "has access" to a namespace #1857

Merged
merged 5 commits into from
Apr 4, 2022

Conversation

jpellizzari
Copy link
Contributor

@jpellizzari jpellizzari commented Mar 31, 2022

Closes #1725

The goal of this work was to be able to provide a list of Kubernetes namespaces that a user can access. "Access" in this context, is determined by a minimum set of actions a user can take. In the default case, those actions map directly to the things that the wego-app can do.

Some thoughts:

  • This feels wrong: is there not already some k8s machinery that can do this? I looked at lots of different source code, but could not find anything that does exactly this.
  • The hasAllRules function is very un-go like to me, but given how flexible the rules can be, I chose to model this as a string-matching problem. Any input on that section would be appreciated.

CC @JamWils This is the brute force approach we had discussed, with an opening for a different (better?) implementation in the future.

@jpellizzari jpellizzari force-pushed the 1725-ns-list branch 3 times, most recently from 20f20c6 to db074a1 Compare March 31, 2022 18:22
@jpellizzari jpellizzari changed the title 1725 ns list @jpellizzari Add nsaccess package to determine if a user "has access" to a namespace Mar 31, 2022
@jpellizzari jpellizzari added the type/enhancement New feature or request label Mar 31, 2022
@jpellizzari jpellizzari marked this pull request as ready for review March 31, 2022 18:35
@jpellizzari jpellizzari changed the title @jpellizzari Add nsaccess package to determine if a user "has access" to a namespace Add nsaccess package to determine if a user "has access" to a namespace Mar 31, 2022
Copy link
Contributor

@yitsushi yitsushi left a comment

Choose a reason for hiding this comment

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

a small nit, otherwise lgtm

(hasAllRules is a bit chaotic)

Comment on lines 135 to 141
func newNamespace(ctx context.Context, k client.Client, g *GomegaWithT) corev1.Namespace {
ns := &corev1.Namespace{}
ns.Name = "kube-test-" + rand.String(5)

g.Expect(k.Create(ctx, ns)).To(Succeed())

return ns
return *ns
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be

func newNamespace(ctx context.Context, k client.Client, g *GomegaWithT) corev1.Namespace {
	ns := corev1.Namespace{}
	ns.Name = "kube-test-" + rand.String(5)

	g.Expect(k.Create(ctx, &ns)).To(Succeed())

	return ns
}

It's kind of the same, but looks less weird with return *ns.

Copy link
Contributor

@luizbafilho luizbafilho left a comment

Choose a reason for hiding this comment

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

Minor comment to help out clean up the tests

}

createRole(t, adminClient, roleName, rules)
secretName := createKubeConfig(t, adminClient, testCfg)
Copy link
Contributor

Choose a reason for hiding this comment

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

we can simplify the user config creation by just:

userCfg := *testCfg

userCfg.Impersonate = rest.ImpersonationConfig{
	UserName: userName,
}

@jpellizzari
Copy link
Contributor Author

TODO:

	t.Run("works when a user has * permissions on a resource", func(t *testing.T) {

	})

@jpellizzari jpellizzari merged commit 2bf6fcc into main Apr 4, 2022
@jpellizzari jpellizzari deleted the 1725-ns-list branch April 4, 2022 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return a list of namespaces that a user can access
3 participants