From 2fee0704e490e1bda5237b64f0d648e05e26d562 Mon Sep 17 00:00:00 2001 From: zoetrope Date: Fri, 17 May 2024 10:07:31 +0000 Subject: [PATCH 1/3] Support extra parameters in the templates --- api/v1beta1/tenant_types.go | 4 ++ api/v1beta1/zz_generated.deepcopy.go | 7 +++ charts/cattage/crds/tenant.yaml | 5 ++ .../crd/bases/cattage.cybozu.io_tenants.yaml | 6 +++ config/manager/configmap.yaml | 8 ++-- config/samples/tenant.yaml | 4 ++ controllers/tenant_controller.go | 48 +++++++++++++++---- controllers/tenant_controller_test.go | 39 ++++++++++++--- controllers/testdata/appprojecttemplate.yaml | 4 +- controllers/testdata/rolebindingtemplate.yaml | 2 +- docs/crd_tenant.md | 1 + 11 files changed, 104 insertions(+), 24 deletions(-) diff --git a/api/v1beta1/tenant_types.go b/api/v1beta1/tenant_types.go index 8c28ff6..ab3d83e 100644 --- a/api/v1beta1/tenant_types.go +++ b/api/v1beta1/tenant_types.go @@ -23,6 +23,10 @@ type TenantSpec struct { // If not specified, the default controller is used. // +optional ControllerName string `json:"controllerName,omitempty"` + + // ExtraParams is a map of extra parameters that can be used in the templates. + // +optional + ExtraParams map[string]string `json:"extraParams,omitempty"` } // RootNamespaceSpec defines the desired state of Namespace. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 88da99c..645dbe5 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -155,6 +155,13 @@ func (in *TenantSpec) DeepCopyInto(out *TenantSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.ExtraParams != nil { + in, out := &in.ExtraParams, &out.ExtraParams + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TenantSpec. diff --git a/charts/cattage/crds/tenant.yaml b/charts/cattage/crds/tenant.yaml index ce094e8..f4bd0b4 100644 --- a/charts/cattage/crds/tenant.yaml +++ b/charts/cattage/crds/tenant.yaml @@ -77,6 +77,11 @@ spec: - roles type: object type: array + extraParams: + additionalProperties: + type: string + description: ExtraParams is a map of extra parameters that can be used in the templates. + type: object rootNamespaces: description: RootNamespaces are the list of root namespaces that belong to this tenant. items: diff --git a/config/crd/bases/cattage.cybozu.io_tenants.yaml b/config/crd/bases/cattage.cybozu.io_tenants.yaml index 6c9dc30..8760070 100644 --- a/config/crd/bases/cattage.cybozu.io_tenants.yaml +++ b/config/crd/bases/cattage.cybozu.io_tenants.yaml @@ -79,6 +79,12 @@ spec: - roles type: object type: array + extraParams: + additionalProperties: + type: string + description: ExtraParams is a map of extra parameters that can be + used in the templates. + type: object rootNamespaces: description: RootNamespaces are the list of root namespaces that belong to this tenant. diff --git a/config/manager/configmap.yaml b/config/manager/configmap.yaml index 63f7b59..48094da 100644 --- a/config/manager/configmap.yaml +++ b/config/manager/configmap.yaml @@ -25,9 +25,9 @@ data: {{- range .Roles.admin }} - apiGroup: rbac.authorization.k8s.io kind: Group - name: {{ . }} + name: {{ .Name }} - kind: ServiceAccount - name: node-{{ . }} + name: node-{{ .Name }} namespace: teleport {{- end }} argocd: @@ -48,9 +48,9 @@ data: kind: LimitRange roles: - groups: - - cybozu-go:{{ .Name }} + - cybozu-go:{{with .Extras.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- range .Roles.admin }} - - cybozu-go:{{ . }} + - cybozu-go:{{with .Extras.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- end }} name: admin policies: diff --git a/config/samples/tenant.yaml b/config/samples/tenant.yaml index bb85a3f..397deef 100644 --- a/config/samples/tenant.yaml +++ b/config/samples/tenant.yaml @@ -6,6 +6,8 @@ spec: rootNamespaces: - name: app-a controllerName: second + extraParams: + GitHubTeam: a-team-gh --- apiVersion: cattage.cybozu.io/v1beta1 kind: Tenant @@ -21,3 +23,5 @@ spec: - name: a-team roles: - admin + extraParams: + GitHubTeam: b-team-gh diff --git a/controllers/tenant_controller.go b/controllers/tenant_controller.go index 22d26d2..c48ead9 100644 --- a/controllers/tenant_controller.go +++ b/controllers/tenant_controller.go @@ -354,15 +354,23 @@ func (r *TenantReconciler) patchRoleBinding(ctx context.Context, rb *acrbacv1.Ro }) } -func rolesMap(delegates []cattagev1beta1.DelegateSpec) map[string][]string { - result := make(map[string][]string) +func (r *TenantReconciler) rolesMap(ctx context.Context, delegates []cattagev1beta1.DelegateSpec) (map[string][]Role, error) { + result := make(map[string][]Role) for _, d := range delegates { for _, role := range d.Roles { - result[role] = append(result[role], d.Name) + delegatedTenant := &cattagev1beta1.Tenant{} + err := r.client.Get(ctx, client.ObjectKey{Name: d.Name}, delegatedTenant) + if err != nil { + return nil, err + } + result[role] = append(result[role], Role{ + Name: delegatedTenant.Name, + Extras: delegatedTenant.Spec.ExtraParams, + }) } } - return result + return result, nil } func (r *TenantReconciler) reconcileNamespaces(ctx context.Context, tenant *cattagev1beta1.Tenant) error { @@ -395,13 +403,20 @@ func (r *TenantReconciler) reconcileNamespaces(ctx context.Context, tenant *catt if err != nil { return err } + roles, err := r.rolesMap(ctx, tenant.Spec.Delegates) + if err != nil { + return err + } + var buf bytes.Buffer err = tpl.Execute(&buf, struct { - Name string - Roles map[string][]string + Name string + Roles map[string][]Role + Extras map[string]string }{ - Name: tenant.Name, - Roles: rolesMap(tenant.Spec.Delegates), + Name: tenant.Name, + Roles: roles, + Extras: tenant.Spec.ExtraParams, }) if err != nil { return err @@ -445,6 +460,11 @@ func (r *TenantReconciler) reconcileNamespaces(ctx context.Context, tenant *catt return nil } +type Role struct { + Name string + Extras map[string]string +} + func (r *TenantReconciler) reconcileArgoCD(ctx context.Context, tenant *cattagev1beta1.Tenant) error { logger := log.FromContext(ctx) @@ -469,17 +489,24 @@ func (r *TenantReconciler) reconcileArgoCD(ctx context.Context, tenant *cattagev return err } + roles, err := r.rolesMap(ctx, tenant.Spec.Delegates) + if err != nil { + return err + } + var buf bytes.Buffer err = tpl.Execute(&buf, struct { Name string Namespaces []string - Roles map[string][]string + Roles map[string][]Role Repositories []string + Extras map[string]string }{ Name: tenant.Name, Namespaces: namespaces, - Roles: rolesMap(tenant.Spec.Delegates), + Roles: roles, Repositories: tenant.Spec.ArgoCD.Repositories, + Extras: tenant.Spec.ExtraParams, }) if err != nil { return err @@ -489,6 +516,7 @@ func (r *TenantReconciler) reconcileArgoCD(ctx context.Context, tenant *cattagev dec := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme) _, _, err = dec.Decode(buf.Bytes(), nil, proj) if err != nil { + logger.Error(err, "failed to decode", "yaml", buf.String()) return err } diff --git a/controllers/tenant_controller_test.go b/controllers/tenant_controller_test.go index 9aa841c..5b0a651 100644 --- a/controllers/tenant_controller_test.go +++ b/controllers/tenant_controller_test.go @@ -90,7 +90,29 @@ var _ = Describe("Tenant controller", Ordered, func() { }) It("should create root namespaces, rolebindings and an appproject", func() { - tenant := &cattagev1beta1.Tenant{ + cTeam := &cattagev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-team", + }, + Spec: cattagev1beta1.TenantSpec{ + RootNamespaces: []cattagev1beta1.RootNamespaceSpec{ + { + Name: "app-c", + }, + }, + ArgoCD: cattagev1beta1.ArgoCDSpec{ + Repositories: []string{ + "https://github.com/cybozu-go/*", + }, + }, + ExtraParams: map[string]string{ + "GitHubTeam": "c-team-gh", + }, + }, + } + err := k8sClient.Create(ctx, cTeam) + Expect(err).ToNot(HaveOccurred()) + xTeam := &cattagev1beta1.Tenant{ ObjectMeta: metav1.ObjectMeta{ Name: "x-team", }, @@ -119,9 +141,12 @@ var _ = Describe("Tenant controller", Ordered, func() { }, }, }, + ExtraParams: map[string]string{ + "GitHubTeam": "x-team-gh", + }, }, } - err := k8sClient.Create(ctx, tenant) + err = k8sClient.Create(ctx, xTeam) Expect(err).ToNot(HaveOccurred()) ns := &corev1.Namespace{} @@ -198,7 +223,7 @@ var _ = Describe("Tenant controller", Ordered, func() { }), "roles": ConsistOf( MatchAllKeys(Keys{ - "groups": ConsistOf("cybozu-go:x-team", "cybozu-go:c-team"), + "groups": ConsistOf("cybozu-go:x-team-gh", "cybozu-go:c-team-gh"), "name": Equal("admin"), "policies": ConsistOf("p, proj:x-team:admin, applications, *, x-team/*, allow"), }), @@ -214,14 +239,14 @@ var _ = Describe("Tenant controller", Ordered, func() { err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "argocd", Name: "all-tenant-namespaces-cm"}, allNsCm) g.Expect(err).NotTo(HaveOccurred()) allNs := strings.Split(allNsCm.Data["application.namespaces"], ",") - g.Expect(allNs).Should(ConsistOf("app-x", "sub-4")) + g.Expect(allNs).Should(ConsistOf("app-x", "sub-4", "app-c")) }).Should(Succeed()) Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "argocd", Name: "default-application-controller-cm"}, defaultCm) g.Expect(err).NotTo(HaveOccurred()) defaultNs := strings.Split(defaultCm.Data["application.namespaces"], ",") - g.Expect(defaultNs).Should(ConsistOf("app-x", "sub-4")) + g.Expect(defaultNs).Should(ConsistOf("app-x", "sub-4", "app-c")) }).Should(Succeed()) tenantS := &cattagev1beta1.Tenant{ @@ -245,13 +270,13 @@ var _ = Describe("Tenant controller", Ordered, func() { err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "argocd", Name: "all-tenant-namespaces-cm"}, allNsCm) g.Expect(err).NotTo(HaveOccurred()) allNs := strings.Split(allNsCm.Data["application.namespaces"], ",") - g.Expect(allNs).Should(ConsistOf("app-x", "sub-4", "app-a", "sub-1", "sub-2", "sub-3")) + g.Expect(allNs).Should(ConsistOf("app-x", "sub-4", "app-a", "sub-1", "sub-2", "sub-3", "app-c")) }).Should(Succeed()) Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "argocd", Name: "default-application-controller-cm"}, defaultCm) g.Expect(err).NotTo(HaveOccurred()) defaultNs := strings.Split(defaultCm.Data["application.namespaces"], ",") - g.Expect(defaultNs).Should(ConsistOf("app-x", "sub-4")) + g.Expect(defaultNs).Should(ConsistOf("app-x", "sub-4", "app-c")) }).Should(Succeed()) Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "argocd", Name: "second-application-controller-cm"}, secondCm) diff --git a/controllers/testdata/appprojecttemplate.yaml b/controllers/testdata/appprojecttemplate.yaml index ffead51..1af6849 100644 --- a/controllers/testdata/appprojecttemplate.yaml +++ b/controllers/testdata/appprojecttemplate.yaml @@ -15,9 +15,9 @@ spec: warn: false roles: - groups: - - cybozu-go:{{ .Name }} + - cybozu-go:{{with .Extras.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- range .Roles.admin }} - - cybozu-go:{{ . }} + - cybozu-go:{{with .Extras.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- end }} name: admin policies: diff --git a/controllers/testdata/rolebindingtemplate.yaml b/controllers/testdata/rolebindingtemplate.yaml index df13553..05ee110 100644 --- a/controllers/testdata/rolebindingtemplate.yaml +++ b/controllers/testdata/rolebindingtemplate.yaml @@ -11,5 +11,5 @@ subjects: {{- range .Roles.admin }} - apiGroup: rbac.authorization.k8s.io kind: Group - name: {{ . }} + name: {{ .Name }} {{- end }} diff --git a/docs/crd_tenant.md b/docs/crd_tenant.md index 2705dcb..1475845 100644 --- a/docs/crd_tenant.md +++ b/docs/crd_tenant.md @@ -78,6 +78,7 @@ TenantSpec defines the desired state of Tenant. | argocd | ArgoCD is the settings of Argo CD for this tenant. | [ArgoCDSpec](#argocdspec) | false | | delegates | Delegates is a list of other tenants that are delegated access to this tenant. | [][DelegateSpec](#delegatespec) | false | | controllerName | ControllerName is the name of the application-controller that manages this tenant's applications. If not specified, the default controller is used. | string | false | +| extraParams | ExtraParams is a map of extra parameters that can be used in the templates. | map[string]string | false | [Back to Custom Resources](#custom-resources) From 29e38a58c997947deda3c73a2a16037ebb9e985d Mon Sep 17 00:00:00 2001 From: zoetrope Date: Mon, 20 May 2024 05:08:01 +0000 Subject: [PATCH 2/3] Support extra parameters in the templates --- config/manager/configmap.yaml | 4 +-- controllers/tenant_controller.go | 24 +++++++------- controllers/testdata/appprojecttemplate.yaml | 4 +-- docs/config.md | 34 ++++++++++++-------- 4 files changed, 37 insertions(+), 29 deletions(-) diff --git a/config/manager/configmap.yaml b/config/manager/configmap.yaml index 48094da..9155862 100644 --- a/config/manager/configmap.yaml +++ b/config/manager/configmap.yaml @@ -48,9 +48,9 @@ data: kind: LimitRange roles: - groups: - - cybozu-go:{{with .Extras.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} + - cybozu-go:{{with .ExtraParams.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- range .Roles.admin }} - - cybozu-go:{{with .Extras.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} + - cybozu-go:{{with .ExtraParams.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- end }} name: admin policies: diff --git a/controllers/tenant_controller.go b/controllers/tenant_controller.go index c48ead9..b9382bf 100644 --- a/controllers/tenant_controller.go +++ b/controllers/tenant_controller.go @@ -365,8 +365,8 @@ func (r *TenantReconciler) rolesMap(ctx context.Context, delegates []cattagev1be return nil, err } result[role] = append(result[role], Role{ - Name: delegatedTenant.Name, - Extras: delegatedTenant.Spec.ExtraParams, + Name: delegatedTenant.Name, + ExtraParams: delegatedTenant.Spec.ExtraParams, }) } } @@ -410,13 +410,13 @@ func (r *TenantReconciler) reconcileNamespaces(ctx context.Context, tenant *catt var buf bytes.Buffer err = tpl.Execute(&buf, struct { - Name string - Roles map[string][]Role - Extras map[string]string + Name string + Roles map[string][]Role + ExtraParams map[string]string }{ - Name: tenant.Name, - Roles: roles, - Extras: tenant.Spec.ExtraParams, + Name: tenant.Name, + Roles: roles, + ExtraParams: tenant.Spec.ExtraParams, }) if err != nil { return err @@ -461,8 +461,8 @@ func (r *TenantReconciler) reconcileNamespaces(ctx context.Context, tenant *catt } type Role struct { - Name string - Extras map[string]string + Name string + ExtraParams map[string]string } func (r *TenantReconciler) reconcileArgoCD(ctx context.Context, tenant *cattagev1beta1.Tenant) error { @@ -500,13 +500,13 @@ func (r *TenantReconciler) reconcileArgoCD(ctx context.Context, tenant *cattagev Namespaces []string Roles map[string][]Role Repositories []string - Extras map[string]string + ExtraParams map[string]string }{ Name: tenant.Name, Namespaces: namespaces, Roles: roles, Repositories: tenant.Spec.ArgoCD.Repositories, - Extras: tenant.Spec.ExtraParams, + ExtraParams: tenant.Spec.ExtraParams, }) if err != nil { return err diff --git a/controllers/testdata/appprojecttemplate.yaml b/controllers/testdata/appprojecttemplate.yaml index 1af6849..360a7d5 100644 --- a/controllers/testdata/appprojecttemplate.yaml +++ b/controllers/testdata/appprojecttemplate.yaml @@ -15,9 +15,9 @@ spec: warn: false roles: - groups: - - cybozu-go:{{with .Extras.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} + - cybozu-go:{{with .ExtraParams.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- range .Roles.admin }} - - cybozu-go:{{with .Extras.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} + - cybozu-go:{{with .ExtraParams.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- end }} name: admin policies: diff --git a/docs/config.md b/docs/config.md index bdd504a..34a442b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -38,7 +38,7 @@ namespace: {{- range .Roles.admin }} - apiGroup: rbac.authorization.k8s.io kind: Group - name: {{ . }} + name: {{ .Name }} {{- end }} argocd: namespace: argocd @@ -53,9 +53,10 @@ argocd: {{- end }} roles: - groups: - - {{ .Name }} + # If `GitHubName` is specified as `ExtraParams`, use it, otherwise use `Name`. + - {{with .ExtraParams.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- range .Roles.admin }} - - {{ . }} + - {{with .ExtraParams.GitHubTeam}}{{ . }}{{else}}{{ .Name }}{{end}} {{- end }} name: admin policies: @@ -76,19 +77,26 @@ argocd: `roleBindingTemplate` can use the following variables: -| Key | Type | Description | -|---------|-----------------------|----------------------------------------------------------------------------------| -| `Name` | `string` | The name of the tenant. | -| `Roles` | `map[string][]string` | Map of other tenants that are accessible to this tenant. The key is a role name. | +| Key | Type | Description | +|---------------|---------------------|----------------------------------------------------------------------------------| +| `Name` | `string` | The name of the tenant. | +| `Roles` | `map[string]Role` | Map of other tenants that are accessible to this tenant. The key is a role name. | +| `ExtraParams` | `map[string]string` | Extra parameters specified per tenant. | `appProjectTemplate` can use the following variables: -| Key | Type | Description | -|----------------|-----------------------|----------------------------------------------------------------------------------| -| `Name` | `string` | The name of the tenant. | -| `Namespaces` | `[]string` | List of namespaces belonging to a tenant (including sub-namespaces). | -| `Repositories` | `[]string` | List of repository URLs which can be used by the tenant. | -| `Roles` | `map[string][]string` | Map of other tenants that are accessible to this tenant. The key is a role name. | +| Key | Type | Description | +|----------------|---------------------|----------------------------------------------------------------------------------| +| `Name` | `string` | The name of the tenant. | +| `Namespaces` | `[]string` | List of namespaces belonging to a tenant (including sub-namespaces). | +| `Repositories` | `[]string` | List of repository URLs which can be used by the tenant. | +| `Roles` | `map[string]Role` | Map of other tenants that are accessible to this tenant. The key is a role name. | +| `ExtraParams` | `map[string]string` | Extra parameters specified per tenant. | + +| Key | Type | Description | +|---------------|---------------------|----------------------------------------| +| `Name` | `string` | The name of the tenant. | +| `ExtraParams` | `map[string]string` | Extra parameters specified per tenant. | ## Environment variables From 43f96c3645e861094a25f3193e3ec23aeea49ac6 Mon Sep 17 00:00:00 2001 From: zoetrope Date: Wed, 22 May 2024 09:55:44 +0000 Subject: [PATCH 3/3] Reflect review comments --- charts/cattage/values.yaml | 4 ++-- docs/setup.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/charts/cattage/values.yaml b/charts/cattage/values.yaml index b67c014..52c2ec3 100644 --- a/charts/cattage/values.yaml +++ b/charts/cattage/values.yaml @@ -44,7 +44,7 @@ controller: {{- range .Roles.admin }} - apiGroup: rbac.authorization.k8s.io kind: Group - name: {{ . }} + name: {{ .Name }} {{- end }} argocd: namespace: argocd @@ -66,7 +66,7 @@ controller: - groups: - cybozu-go:{{ .Name }} {{- range .Roles.admin }} - - cybozu-go:{{ . }} + - cybozu-go:{{ .Name }} {{- end }} name: admin policies: diff --git a/docs/setup.md b/docs/setup.md index 59d74a7..334a2a2 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -111,7 +111,7 @@ controller: {{- range .Roles.admin }} - apiGroup: rbac.authorization.k8s.io kind: Group - name: {{ . }} + name: {{ .Name }} {{- end }} argocd: namespace: argocd @@ -128,7 +128,7 @@ controller: - groups: - {{ .Name }} {{- range .Roles.admin }} - - {{ . }} + - {{ .Name }} {{- end }} name: admin policies: