Skip to content

Commit 350b6c7

Browse files
oliverbaehleroliverbaehler
authored
feat(controller): drop most legacy routes when ProxyClusterScoped is active (#674)
* chore: add golint and fix truthy Signed-off-by: Oliver Bähler <[email protected]> * fix(api): deprecate operations field Signed-off-by: Oliver Bähler <[email protected]> * test(e2e): add header test Signed-off-by: Oliver Bähler <[email protected]> * fix(controller): register dynamic clsuterresource paths for legacy proxysettings when enabled Signed-off-by: Oliver Bähler <[email protected]> * test(e2e): register dynamic clsuterresource paths for legacy proxysettings when enabled Signed-off-by: Oliver Bähler <[email protected]> --------- Signed-off-by: Oliver Bähler <[email protected]>
1 parent ca872ea commit 350b6c7

File tree

13 files changed

+273
-50
lines changed

13 files changed

+273
-50
lines changed

‎.github/configs/lintconf.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ ignore:
66
rules:
77
truthy:
88
level: warning
9+
allowed-values:
10+
- "true"
11+
- "false"
12+
- "on"
13+
- "off"
914
check-keys: false
1015
braces:
1116
min-spaces-inside: 0

‎.pre-commit-config.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,8 @@ repos:
3434
entry: make helm-lint
3535
language: system
3636
files: ^charts/
37+
- id: golangci-lint
38+
name: Execute golangci-lint
39+
entry: make golint
40+
language: system
41+
files: \.go$

‎api/v1beta1/clusterresoure.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ type ClusterResource struct {
2222
Resources []string `json:"resources"`
2323

2424
// Operations which can be executed on the selected resources.
25-
// +kubebuilder:default={List}
26-
Operations []ClusterResourceOperation `json:"operations"`
25+
// Deprecated: For all registered Routes only LIST ang GET requests will intercepted
26+
// Other permissions must be implemented via kubernetes native RBAC
27+
Operations []ClusterResourceOperation `json:"operations,omitempty"`
2728

2829
// Select all cluster scoped resources with the given label selector.
2930
// Defining a selector which does not match any resources is considered not selectable (eg. using operation NotExists).

‎charts/capsule-proxy/crds/capsule.clastix.io_globalproxysettings.yaml‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ spec:
6161
type: string
6262
type: array
6363
operations:
64-
default:
65-
- List
66-
description: Operations which can be executed on the selected
67-
resources.
64+
description: |-
65+
Operations which can be executed on the selected resources.
66+
Deprecated: For all registered Routes only LIST ang GET requests will intercepted
67+
Other permissions must be implemented via kubernetes native RBAC
6868
items:
6969
enum:
7070
- List
@@ -128,7 +128,6 @@ spec:
128128
x-kubernetes-map-type: atomic
129129
required:
130130
- apiGroups
131-
- operations
132131
- resources
133132
- selector
134133
type: object

‎charts/capsule-proxy/crds/capsule.clastix.io_proxysettings.yaml‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ spec:
5959
type: string
6060
type: array
6161
operations:
62-
default:
63-
- List
64-
description: Operations which can be executed on the selected
65-
resources.
62+
description: |-
63+
Operations which can be executed on the selected resources.
64+
Deprecated: For all registered Routes only LIST ang GET requests will intercepted
65+
Other permissions must be implemented via kubernetes native RBAC
6666
items:
6767
enum:
6868
- List
@@ -126,7 +126,6 @@ spec:
126126
x-kubernetes-map-type: atomic
127127
required:
128128
- apiGroups
129-
- operations
130129
- resources
131130
- selector
132131
type: object

‎e2e/global_settings_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,4 +161,49 @@ var _ = Describe("GlobalProxySettings", func() {
161161
return listClusterRoles(bobClient)
162162
}).Should(Equal(expectedRoles), "Bob should only have access to the specified cluster roles")
163163
})
164+
165+
It("Should only allow listing clusterroles, but deny create, update, delete", func() {
166+
roleToCreate := &rbacv1.ClusterRole{
167+
ObjectMeta: metav1.ObjectMeta{
168+
Name: "unauthorized-clusterrole",
169+
Labels: e2eLabels(),
170+
},
171+
Rules: []rbacv1.PolicyRule{
172+
{
173+
APIGroups: []string{""},
174+
Resources: []string{"pods"},
175+
Verbs: []string{"get"},
176+
},
177+
},
178+
}
179+
180+
attemptCreate := func(clientset *kubernetes.Clientset) error {
181+
_, err := clientset.RbacV1().ClusterRoles().Create(context.Background(), roleToCreate, metav1.CreateOptions{})
182+
return err
183+
}
184+
185+
attemptUpdate := func(clientset *kubernetes.Clientset) error {
186+
role, err := clientset.RbacV1().ClusterRoles().Get(context.Background(), "tenant-viewer", metav1.GetOptions{})
187+
if err != nil {
188+
return err
189+
}
190+
role.Annotations = map[string]string{"updated": "true"}
191+
_, err = clientset.RbacV1().ClusterRoles().Update(context.Background(), role, metav1.UpdateOptions{})
192+
return err
193+
}
194+
195+
attemptDelete := func(clientset *kubernetes.Clientset) error {
196+
return clientset.RbacV1().ClusterRoles().Delete(context.Background(), "tenant-viewer", metav1.DeleteOptions{})
197+
}
198+
199+
By("Denying create/update/delete for Alice")
200+
Expect(attemptCreate(aliceClient)).To(HaveOccurred(), "Alice should not be able to create ClusterRoles")
201+
Expect(attemptUpdate(aliceClient)).To(HaveOccurred(), "Alice should not be able to update ClusterRoles")
202+
Expect(attemptDelete(aliceClient)).To(HaveOccurred(), "Alice should not be able to delete ClusterRoles")
203+
204+
By("Denying create/update/delete for Bob")
205+
Expect(attemptCreate(bobClient)).To(HaveOccurred(), "Bob should not be able to create ClusterRoles")
206+
Expect(attemptUpdate(bobClient)).To(HaveOccurred(), "Bob should not be able to update ClusterRoles")
207+
Expect(attemptDelete(bobClient)).To(HaveOccurred(), "Bob should not be able to delete ClusterRoles")
208+
})
164209
})

‎e2e/namespace_test.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package e2e_test
2+
3+
import (
4+
"context"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/client-go/kubernetes"
10+
11+
capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
12+
)
13+
14+
var _ = Describe("Namespaces", func() {
15+
var aliceClient, bobClient *kubernetes.Clientset
16+
17+
// Create Global Proxy Settings
18+
wind := &capsulev1beta2.Tenant{
19+
ObjectMeta: metav1.ObjectMeta{
20+
Name: "wind",
21+
Labels: e2eLabels(),
22+
},
23+
Spec: capsulev1beta2.TenantSpec{
24+
Owners: capsulev1beta2.OwnerListSpec{
25+
{
26+
Name: "alice",
27+
Kind: "User",
28+
},
29+
},
30+
},
31+
}
32+
33+
// Create Global Proxy Settings
34+
solar := &capsulev1beta2.Tenant{
35+
ObjectMeta: metav1.ObjectMeta{
36+
Name: "solar",
37+
Labels: e2eLabels(),
38+
},
39+
Spec: capsulev1beta2.TenantSpec{
40+
Owners: capsulev1beta2.OwnerListSpec{
41+
{
42+
Name: "bob",
43+
Kind: "User",
44+
},
45+
{
46+
Name: "alice",
47+
Kind: "User",
48+
},
49+
},
50+
},
51+
}
52+
53+
BeforeEach(func() {
54+
var err error
55+
56+
aliceClient, err = loadKubeConfig("alice")
57+
Expect(err).ToNot(HaveOccurred())
58+
bobClient, err = loadKubeConfig("bob")
59+
Expect(err).ToNot(HaveOccurred())
60+
61+
for _, tnt := range []*capsulev1beta2.Tenant{solar, wind} {
62+
Eventually(func() error {
63+
tnt.ResourceVersion = ""
64+
65+
return k8sClient.Create(context.TODO(), tnt)
66+
}).Should(Succeed())
67+
}
68+
})
69+
70+
JustAfterEach(func() {
71+
for _, tnt := range []*capsulev1beta2.Tenant{solar, wind} {
72+
Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
73+
}
74+
})
75+
76+
It("Should correctly list", func() {
77+
nsAlice1 := NewNamespace("")
78+
nsAlice1.Labels = map[string]string{
79+
"capsule.clastix.io/tenant": "wind",
80+
}
81+
NamespaceCreation(nsAlice1, wind.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
82+
83+
nsAlice2 := NewNamespace("")
84+
nsAlice2.Labels = map[string]string{
85+
"capsule.clastix.io/tenant": "wind",
86+
}
87+
NamespaceCreation(nsAlice2, wind.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
88+
89+
listNamespaces := func(clientset *kubernetes.Clientset) ([]string, error) {
90+
ns, err := clientset.CoreV1().Namespaces().List(context.Background(), metav1.ListOptions{})
91+
if err != nil {
92+
return nil, err
93+
}
94+
var nsNames []string
95+
for _, name := range ns.Items {
96+
nsNames = append(nsNames, name.Name)
97+
}
98+
99+
return nsNames, nil
100+
}
101+
102+
Eventually(func() ([]string, error) {
103+
return listNamespaces(aliceClient)
104+
}).Should(ConsistOf(nsAlice1.GetName(), nsAlice2.GetName()), "Alice should only have access to the expected namespaces, order does not matter")
105+
106+
nsBob1 := NewNamespace("")
107+
nsBob1.Labels = map[string]string{
108+
"capsule.clastix.io/tenant": "solar",
109+
}
110+
NamespaceCreation(nsBob1, solar.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
111+
112+
nsBob2 := NewNamespace("")
113+
nsBob2.Labels = map[string]string{
114+
"capsule.clastix.io/tenant": "solar",
115+
}
116+
NamespaceCreation(nsBob2, solar.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
117+
118+
Eventually(func() ([]string, error) {
119+
return listNamespaces(bobClient)
120+
}).Should(ConsistOf(nsBob1.GetName(), nsBob2.GetName()), "Alice should only have access to the expected namespaces, order does not matter")
121+
122+
})
123+
})

‎e2e/suite_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package e2e_test
44
import (
55
. "github.com/onsi/ginkgo/v2"
66
. "github.com/onsi/gomega"
7+
capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
78
"k8s.io/client-go/rest"
89
"k8s.io/kubectl/pkg/scheme"
910
"k8s.io/utils/ptr"
@@ -36,6 +37,7 @@ var _ = BeforeSuite(func() {
3637
Expect(cfg).ToNot(BeNil())
3738

3839
Expect(v1beta1.AddToScheme(scheme.Scheme)).NotTo(HaveOccurred())
40+
Expect(capsulev1beta2.AddToScheme(scheme.Scheme)).NotTo(HaveOccurred())
3941

4042
ctrlClient, err := client.New(cfg, client.Options{Scheme: scheme.Scheme})
4143
Expect(err).ToNot(HaveOccurred())

‎e2e/utils_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,16 @@ import (
66
"path/filepath"
77
"time"
88

9+
. "github.com/onsi/gomega"
10+
capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
11+
corev1 "k8s.io/api/core/v1"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
913
"k8s.io/apimachinery/pkg/labels"
14+
"k8s.io/apimachinery/pkg/util/rand"
1015
"k8s.io/client-go/kubernetes"
1116
"k8s.io/client-go/tools/clientcmd"
1217
"sigs.k8s.io/controller-runtime/pkg/client"
18+
"sigs.k8s.io/controller-runtime/pkg/client/config"
1319
)
1420

1521
const (
@@ -65,3 +71,40 @@ func loadKubeConfig(user string) (*kubernetes.Clientset, error) {
6571

6672
return clientset, nil
6773
}
74+
75+
func NewNamespace(name string, labels ...map[string]string) *corev1.Namespace {
76+
if len(name) == 0 {
77+
name = rand.String(10)
78+
}
79+
80+
var namespaceLabels map[string]string
81+
if len(labels) > 0 {
82+
namespaceLabels = labels[0]
83+
}
84+
85+
return &corev1.Namespace{
86+
ObjectMeta: metav1.ObjectMeta{
87+
Name: name,
88+
Labels: namespaceLabels,
89+
},
90+
}
91+
}
92+
93+
func NamespaceCreation(ns *corev1.Namespace, owner capsulev1beta2.OwnerSpec, timeout time.Duration) AsyncAssertion {
94+
cs := ownerClient(owner)
95+
return Eventually(func() (err error) {
96+
_, err = cs.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{})
97+
return
98+
}, timeout, defaultPollInterval)
99+
}
100+
101+
func ownerClient(owner capsulev1beta2.OwnerSpec) (cs kubernetes.Interface) {
102+
c, err := config.GetConfig()
103+
Expect(err).ToNot(HaveOccurred())
104+
c.Impersonate.Groups = []string{"projectcapsule.dev", owner.Name}
105+
c.Impersonate.UserName = owner.Name
106+
cs, err = kubernetes.NewForConfig(c)
107+
Expect(err).ToNot(HaveOccurred())
108+
109+
return cs
110+
}

‎internal/modules/clusterscoped/get.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package clusterscoped
22

33
import (
44
"context"
5+
"net/http"
56

67
"github.com/go-logr/logr"
78
"github.com/gorilla/mux"
@@ -60,14 +61,15 @@ func (g get) Handle(proxyTenants []*tenant.ProxyTenant, proxyRequest request.Req
6061

6162
gvk := utils.GetGVKFromURL(proxyRequest.GetHTTPRequest().URL.Path)
6263

63-
operations, requirements := utils.GetClusterScopeRequirements(gvk, proxyTenants)
64+
_, requirements := utils.GetClusterScopeRequirements(gvk, proxyTenants)
65+
6466
if len(requirements) > 0 {
65-
// Verify if the list operation is allowed
66-
if utils.IsAllowed(operations, httpRequest) {
67+
switch httpRequest.Method {
68+
case http.MethodGet:
6769
return g.handleSelector(httpRequest.Context(), gvk, requirements, mux.Vars(httpRequest)["name"])
70+
default:
71+
return nil, nil
6872
}
69-
70-
return nil, errors.NewNotAllowed(gvk.GroupKind())
7173
}
7274

7375
return

0 commit comments

Comments
 (0)