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

Support Naming Policy #17

Merged
merged 20 commits into from
Nov 11, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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
2 changes: 1 addition & 1 deletion charts/accurate/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.1.0
version: 0.1.1

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
9 changes: 9 additions & 0 deletions charts/accurate/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ controller:
- version: v1
kind: Secret

# controller.config.namingPolicies -- List of nameing policy for SubNamespaces.
# root and match are both regular expressions.
# When a SubNamespace is created in a tree starting from a root namespace and the root namespace's name matches root expression, the SubNamespace name is validated with match expression.
# namingPolicies:
# - root: foo
# match: foo_.*
# - root: bar
# match: bar_.*
Copy link
Member

Choose a reason for hiding this comment

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

If we adopt the proposed spec from @zoetrope, this example should be updated too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an example.


additionalRBAC:
# controller.additionalRBAC.rules -- Specify the RBAC rules to be added to the controller.
# ClusterRole and ClusterRoleBinding are created with the names `{{ release name }}-additional-resources`.
Expand Down
4 changes: 3 additions & 1 deletion cmd/accurate-controller/sub/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ func subMain(ns, addr string, port int) error {
}).SetupWithManager(mgr); err != nil {
return fmt.Errorf("unable to create SubNamespace controller: %w", err)
}
hooks.SetupSubNamespaceWebhook(mgr, dec)
if err = hooks.SetupSubNamespaceWebhook(mgr, dec, cfg.NamingPolicies); err != nil {
return fmt.Errorf("unable to create SubNamespace webhook: %w", err)
}

// Resource propagation controller
for _, res := range watched {
Expand Down
2 changes: 2 additions & 0 deletions config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,5 @@ watches:
kind: RoleBinding
- version: v1
kind: Secret

namingPolicies: []
Copy link
Member

Choose a reason for hiding this comment

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

Please add comments just like in the other fields.

9 changes: 9 additions & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ controller:
kind: RoleBinding
- version: v1
kind: Secret

# controller.config.namingPolicies -- List of nameing policy for SubNamespaces.
# root and match are both regular expressions.
# When a SubNamespace is created in a tree starting from a root namespace and the root namespace's name matches root expression, the SubNamespace name is validated with match expression.
# namingPolicies:
# - root: foo
# match: foo_.*
# - root: bar
# match: bar_.*
Copy link
Member

Choose a reason for hiding this comment

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

Why don't we uncomment these lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uncommented.

<snip>
```

Expand Down
66 changes: 59 additions & 7 deletions hooks/subnamespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import (
"encoding/json"
"fmt"
"net/http"
"regexp"

accuratev1 "github.com/cybozu-go/accurate/api/v1"
"github.com/cybozu-go/accurate/pkg/config"
"github.com/cybozu-go/accurate/pkg/constants"
admissionv1 "k8s.io/api/admission/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -43,11 +45,17 @@ func (m *subNamespaceMutator) Handle(ctx context.Context, req admission.Request)
return admission.PatchResponseFromRaw(req.Object.Raw, data)
}

type NamingPolicyRegexp struct {
Root *regexp.Regexp
Match *regexp.Regexp
}

//+kubebuilder:webhook:path=/validate-accurate-cybozu-com-v1-subnamespace,mutating=false,failurePolicy=fail,sideEffects=None,groups=accurate.cybozu.com,resources=subnamespaces,verbs=create;update,versions=v1,name=vsubnamespace.kb.io,admissionReviewVersions={v1,v1beta1}

type subNamespaceValidator struct {
client.Client
dec *admission.Decoder
dec *admission.Decoder
namingPolicies []NamingPolicyRegexp
}

var _ admission.Handler = &subNamespaceValidator{}
Expand All @@ -67,25 +75,69 @@ func (v *subNamespaceValidator) Handle(ctx context.Context, req admission.Reques
return admission.Errored(http.StatusInternalServerError, err)
}

if ns.Labels[constants.LabelType] == constants.NSTypeRoot || ns.Labels[constants.LabelParent] != "" {
return admission.Allowed("")
if ns.Labels[constants.LabelType] != constants.NSTypeRoot && ns.Labels[constants.LabelParent] == "" {
return admission.Denied(fmt.Sprintf("namespace %s is neither a root nor a sub namespace", ns.Name))
}

ok, err := v.notMatchingNamingPolicy(ctx, sn)
if err != nil {
return admission.Errored(http.StatusInternalServerError, err)
}
if !ok {
return admission.Denied(fmt.Sprintf("namespace %s is not match naming policies", ns.Name))
}

return admission.Allowed("")
}

return admission.Denied(fmt.Sprintf("namespace %s is neither a root nor a sub namespace", ns.Name))
func (v *subNamespaceValidator) notMatchingNamingPolicy(ctx context.Context, sn *accuratev1.SubNamespace) (bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

The regular expression should be pre-compiled when loaded from the configuration file,
so that no error would be returned during execution.

for _, policy := range v.namingPolicies {
if policy.Root.MatchString(sn.Namespace) {
Copy link
Member

@zoetrope zoetrope Oct 7, 2021

Choose a reason for hiding this comment

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

I think sn.Namespace is the parent, not the root.
Is this correct?

if !policy.Match.MatchString(sn.Name) {
return false, nil
}
}
}
return true, nil
}

// SetupSubNamespaceWebhook registers the webhooks for SubNamespace
func SetupSubNamespaceWebhook(mgr manager.Manager, dec *admission.Decoder) {
func SetupSubNamespaceWebhook(mgr manager.Manager, dec *admission.Decoder, namingPolicies []config.NamingPolicy) error {
serv := mgr.GetWebhookServer()

m := &subNamespaceMutator{
dec: dec,
}
serv.Register("/mutate-accurate-cybozu-com-v1-subnamespace", &webhook.Admission{Handler: m})

namingPolicyRegexps, err := compileNamingPolicies(namingPolicies)
Copy link
Member

Choose a reason for hiding this comment

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

compile and validate policies in pkg/config.Config.Validate method.

if err != nil {
return err
}

v := &subNamespaceValidator{
Client: mgr.GetClient(),
dec: dec,
Client: mgr.GetClient(),
dec: dec,
namingPolicies: namingPolicyRegexps,
}
serv.Register("/validate-accurate-cybozu-com-v1-subnamespace", &webhook.Admission{Handler: v})
return nil
}

func compileNamingPolicies(namingPolicies []config.NamingPolicy) ([]NamingPolicyRegexp, error) {
var result []NamingPolicyRegexp
for _, policy := range namingPolicies {
root, err := regexp.Compile(policy.Root)
if err != nil {
return nil, err
}

match, err := regexp.Compile(policy.Match)
if err != nil {
return nil, err
}

result = append(result, NamingPolicyRegexp{Root: root, Match: match})
}
return result, nil
}
64 changes: 64 additions & 0 deletions hooks/subnamespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,68 @@ var _ = Describe("SubNamespace webhook", func() {

Expect(controllerutil.ContainsFinalizer(sn, constants.Finalizer)).To(BeTrue())
})

Context("Naming Policy", func() {
When("the root namespace name is matched some Root Naming Policies", func() {
When("the SubNamespace name is matched to the Root's Match Naming Policy", func() {
It("should allow creation of SubNamespace in a root namespace - pattern1", func() {
ns := &corev1.Namespace{}
ns.Name = "naming-policy-root-1"
ns.Labels = map[string]string{constants.LabelType: constants.NSTypeRoot}
err := k8sClient.Create(ctx, ns)
Expect(err).NotTo(HaveOccurred())

sn := &accuratev1.SubNamespace{}
sn.Namespace = "naming-policy-root-1"
sn.Name = "naming-policy-root-1-child"
err = k8sClient.Create(ctx, sn)
Expect(err).NotTo(HaveOccurred())
})

It("should allow creation of SubNamespace in a root namespace - pattern2", func() {
ns := &corev1.Namespace{}
ns.Name = "root-ns-match-1"
ns.Labels = map[string]string{constants.LabelType: constants.NSTypeRoot}
err := k8sClient.Create(ctx, ns)
Expect(err).NotTo(HaveOccurred())

sn := &accuratev1.SubNamespace{}
sn.Namespace = "root-ns-match-1"
sn.Name = "child-match-1"
err = k8sClient.Create(ctx, sn)
Expect(err).NotTo(HaveOccurred())
})
})

When("the SubNamespace name is not matched to the Root's Match Naming Policy", func() {
It("should deny creation of SubNamespace in a root namespace - pattern1", func() {
ns := &corev1.Namespace{}
ns.Name = "naming-policy-root-2"
ns.Labels = map[string]string{constants.LabelType: constants.NSTypeRoot}
err := k8sClient.Create(ctx, ns)
Expect(err).NotTo(HaveOccurred())

sn := &accuratev1.SubNamespace{}
sn.Namespace = "naming-policy-root-2"
sn.Name = "naming-policy-root-2--child"
err = k8sClient.Create(ctx, sn)
Expect(err).To(HaveOccurred())
})

It("should deny creation of SubNamespace in a root namespace - pattern2", func() {
ns := &corev1.Namespace{}
ns.Name = "root-ns-match-2"
ns.Labels = map[string]string{constants.LabelType: constants.NSTypeRoot}
err := k8sClient.Create(ctx, ns)
Expect(err).NotTo(HaveOccurred())

sn := &accuratev1.SubNamespace{}
sn.Namespace = "root-ns-match-2"
sn.Name = "child-2"
err = k8sClient.Create(ctx, sn)
Expect(err).To(HaveOccurred())
})
})
})
})
})
17 changes: 16 additions & 1 deletion hooks/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
admissionv1beta1 "k8s.io/api/admission/v1beta1"
//+kubebuilder:scaffold:imports
accuratev1 "github.com/cybozu-go/accurate/api/v1"
"github.com/cybozu-go/accurate/pkg/config"
"github.com/cybozu-go/accurate/pkg/indexing"
"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
Expand Down Expand Up @@ -91,7 +92,21 @@ var _ = BeforeSuite(func() {
dec, err := admission.NewDecoder(scheme)
Expect(err).NotTo(HaveOccurred())
SetupNamespaceWebhook(mgr, dec)
SetupSubNamespaceWebhook(mgr, dec)
err = SetupSubNamespaceWebhook(mgr, dec, []config.NamingPolicy{
{
Root: "naming-policy-root-1",
Match: "naming-policy-root-1-child",
},
{
Root: "naming-policy-root-2",
Match: "naming-policy-root-2-child",
},
{
Root: ".+-match-.+",
Match: ".+-match-.+",
},
})
Expect(err).NotTo(HaveOccurred())

go func() {
err = mgr.Start(ctx)
Expand Down
7 changes: 7 additions & 0 deletions pkg/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,18 @@ import (
"sigs.k8s.io/yaml"
)

// NamingPolicy represents the configuration of naming policies for Namespaces these are managed by Accurate.
ymmt2005 marked this conversation as resolved.
Show resolved Hide resolved
type NamingPolicy struct {
ymmt2005 marked this conversation as resolved.
Show resolved Hide resolved
Root string `json:"root"`
Match string `json:"match"`
}

// Config represents the configuration file of Accurate.
type Config struct {
LabelKeys []string `json:"labelKeys,omitempty"`
AnnotationKeys []string `json:"annotationKeys,omitempty"`
Watches []metav1.GroupVersionKind `json:"watches,omitempty"`
NamingPolicies []NamingPolicy `json:"namingPolicies,omitempty"`
}

// Validate validates the configurations.
Expand Down