Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
34 changes: 34 additions & 0 deletions config/manager/psql/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,39 @@ metadata:
data:
useinstanceprincipal: dHJ1ZQ==
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: controller-manager
namespace: system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: manager-rolebinding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: manager-role
subjects:
- kind: ServiceAccount
name: controller-manager
namespace: system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: leader-election-rolebinding
namespace: system
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: leader-election-role
subjects:
- kind: ServiceAccount
name: controller-manager
namespace: system
---
apiVersion: apps/v1
kind: Deployment
metadata:
Expand All @@ -34,6 +67,7 @@ spec:
labels:
control-plane: controller-manager
spec:
serviceAccountName: controller-manager
securityContext:
runAsUser: 65532
containers:
Expand Down
2 changes: 1 addition & 1 deletion controllers/psql/dbsystem_controller.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/api-generator-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Each service record defines:
| `sampleOrder` | Optional deterministic ordering hint for generated sample entries. |
| `packageProfile` | Install posture for the group: `controller-backed` or `crd-only`. |
| `package.extraResources` | Optional extra package overlay resources to include in the generated install kustomization. |
| `package.dedicatedServiceAccount` | When true, generate a package-specific manager ServiceAccount and its manager and leader-election bindings instead of binding those roles to the shared default ServiceAccount. |
| `selection.enabled` | Whether the service participates in the default active generator surface. |
| `selection.mode` | Default selection contract for the service: `all` or `explicit`. |
| `selection.includeKinds` | Optional non-empty kind list used only when `selection.mode=explicit`. |
Expand Down
17 changes: 15 additions & 2 deletions internal/generator/checked_in_rbac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestCheckedInDatabaseAutonomousDatabasePackageRBACMatchesReadOnlySecretSema
func TestCheckedInPSQLDbSystemPackageRBACMatchesSecretAndEventRecorderSemantics(t *testing.T) {
controllerPath := filepath.Join(repoRoot(t), "controllers", "psql", "dbsystem_controller.go")
assertFileContains(t, controllerPath, []string{
`// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch`,
`// +kubebuilder:rbac:groups="",resources=secrets,verbs=get`,
`// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch`,
})
assertFileDoesNotContain(t, controllerPath, []string{
Expand All @@ -52,9 +52,22 @@ func TestCheckedInPSQLDbSystemPackageRBACMatchesSecretAndEventRecorderSemantics(
filepath.Join(repoRoot(t), "packages", "psql", "install", "generated", "rbac", "role.yaml"),
map[string][]string{
"events": {"create", "patch"},
"secrets": {"get", "list", "watch"},
"secrets": {"get"},
},
)

managerPath := filepath.Join(repoRoot(t), "config", "manager", "psql", "manager.yaml")
assertFileContains(t, managerPath, []string{
"kind: ServiceAccount",
"name: controller-manager",
"kind: ClusterRoleBinding",
"kind: RoleBinding",
"serviceAccountName: controller-manager",
})
assertFileDoesNotContain(t, filepath.Join(repoRoot(t), "packages", "psql", "install", "kustomization.yaml"), []string{
"../../../config/rbac/role_binding.yaml",
"../../../config/rbac/leader_election_role_binding.yaml",
})
}

func TestCheckedInRedisClusterPackageRBACMatchesEventRecorderSemantics(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions internal/generator/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ type PackageProfile struct {
// PackageConfig describes service-scoped package overlay details.
type PackageConfig struct {
ExtraResources []string `yaml:"extraResources,omitempty"`
// DedicatedServiceAccount isolates a package manager from the shared default
// service account and its role bindings.
DedicatedServiceAccount bool `yaml:"dedicatedServiceAccount,omitempty"`
}

// SelectionConfig declares whether a service participates in the default active surface.
Expand Down
4 changes: 3 additions & 1 deletion internal/generator/config/services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6734,6 +6734,8 @@ services:
mode: explicit
includeKinds:
- DbSystem
package:
dedicatedServiceAccount: true
generation:
controller:
strategy: generated
Expand All @@ -6752,7 +6754,7 @@ services:
formalClassification: lifecycle
controller:
extraRBACMarkers:
- groups="",resources=secrets,verbs=get;list;watch
- groups="",resources=secrets,verbs=get
specFields:
- name: AdminUsername
type: shared.UsernameSource
Expand Down
5 changes: 4 additions & 1 deletion internal/generator/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2951,10 +2951,13 @@ func assertPSQLRuntimeRolloutMetadata(t *testing.T, service *ServiceConfig) {
if override.Kind != "DbSystem" {
t.Fatalf("psql override kind = %q, want %q", override.Kind, "DbSystem")
}
if !service.Package.DedicatedServiceAccount {
t.Fatal("psql dedicatedServiceAccount = false, want true")
}
if !slices.Equal(
override.Controller.ExtraRBACMarkers,
[]string{
`groups="",resources=secrets,verbs=get;list;watch`,
`groups="",resources=secrets,verbs=get`,
},
) {
t.Fatalf("psql extra RBAC markers = %v, want secret read markers only", override.Controller.ExtraRBACMarkers)
Expand Down
12 changes: 9 additions & 3 deletions internal/generator/package_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,16 @@ func buildPackageOutputModelFor(service ServiceConfig, outputName string, defaul
"generated/crd",
"generated/rbac",
managerOverlay,
"../../../config/rbac/role_binding.yaml",
"../../../config/rbac/leader_election_role.yaml",
"../../../config/rbac/leader_election_role_binding.yaml",
)
if !service.Package.DedicatedServiceAccount {
output.Install.Resources = append(output.Install.Resources,
"../../../config/rbac/role_binding.yaml",
)
}
output.Install.Resources = append(output.Install.Resources, "../../../config/rbac/leader_election_role.yaml")
if !service.Package.DedicatedServiceAccount {
output.Install.Resources = append(output.Install.Resources, "../../../config/rbac/leader_election_role_binding.yaml")
}
output.Install.Resources = appendUniqueStrings(output.Install.Resources, service.Package.ExtraResources...)
if service.WebhookGenerationStrategy() == GenerationStrategyManual {
output.Install.Resources = appendUniqueStrings(output.Install.Resources,
Expand Down
48 changes: 45 additions & 3 deletions internal/generator/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (r *Renderer) RenderManagerOutputs(root string, pkg *PackageModel, overwrit
return err
}

managerDeploymentContent, err := renderManagerDeploymentFile()
managerDeploymentContent, err := renderManagerDeploymentFile(pkg.Service.Package.DedicatedServiceAccount)
if err != nil {
return fmt.Errorf("render manager deployment for %s: %w", pkg.Service.Service, err)
}
Expand Down Expand Up @@ -470,8 +470,12 @@ func renderManagerKustomizationFile() (string, error) {
return executeTemplate(managerKustomizationTemplate, struct{}{})
}

func renderManagerDeploymentFile() (string, error) {
return executeTemplate(managerDeploymentTemplate, struct{}{})
func renderManagerDeploymentFile(dedicatedServiceAccount bool) (string, error) {
return executeTemplate(managerDeploymentTemplate, struct {
DedicatedServiceAccount bool
}{
DedicatedServiceAccount: dedicatedServiceAccount,
})
}

func renderControllerManagerConfigFile(group string) (string, error) {
Expand Down Expand Up @@ -1689,6 +1693,41 @@ metadata:
name: osokconfig
data:
useinstanceprincipal: dHJ1ZQ==
{{- if .DedicatedServiceAccount}}
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: controller-manager
namespace: system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: manager-rolebinding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: manager-role
subjects:
- kind: ServiceAccount
name: controller-manager
namespace: system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: leader-election-rolebinding
namespace: system
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: leader-election-role
subjects:
- kind: ServiceAccount
name: controller-manager
namespace: system
{{- end}}
---
apiVersion: apps/v1
kind: Deployment
Expand All @@ -1707,6 +1746,9 @@ spec:
labels:
control-plane: controller-manager
spec:
{{- if .DedicatedServiceAccount}}
serviceAccountName: controller-manager
{{- end}}
securityContext:
runAsUser: 65532
containers:
Expand Down
2 changes: 0 additions & 2 deletions packages/psql/install/generated/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ rules:
- secrets
verbs:
- get
- list
- watch
- apiGroups:
- psql.oracle.com
resources:
Expand Down
2 changes: 0 additions & 2 deletions packages/psql/install/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ resources:
- generated/crd
- generated/rbac
- ../../../config/manager/psql
- ../../../config/rbac/role_binding.yaml
- ../../../config/rbac/leader_election_role.yaml
- ../../../config/rbac/leader_election_role_binding.yaml

patches:
- path: ../../../config/default/manager_config_patch.yaml
Expand Down
21 changes: 16 additions & 5 deletions pkg/manager/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
Expand Down Expand Up @@ -195,11 +196,7 @@ func Run(opts Options, registrars ...RegisterFunc) error {

metricsClient := metrics.Init(opts.MetricsServiceName, loggerutil.OSOKLogger{Logger: ctrl.Log.WithName("metrics")})

credClient := &kubesecret.KubeSecretClient{
Client: mgr.GetClient(),
Log: loggerutil.OSOKLogger{Logger: ctrl.Log.WithName("credential-helper").WithName("KubeSecretClient")},
Metrics: metricsClient,
}
credClient := newCredentialClient(mgr, metricsClient)

deps := &Dependencies{
Provider: provider,
Expand Down Expand Up @@ -235,6 +232,20 @@ func Run(opts Options, registrars ...RegisterFunc) error {
return nil
}

type credentialClientManager interface {
GetClient() ctrlclient.Client
GetAPIReader() ctrlclient.Reader
}

func newCredentialClient(mgr credentialClientManager, metricsClient *metrics.Metrics) *kubesecret.KubeSecretClient {
return kubesecret.NewWithReader(
mgr.GetClient(),
mgr.GetAPIReader(),
loggerutil.OSOKLogger{Logger: ctrl.Log.WithName("credential-helper").WithName("KubeSecretClient")},
metricsClient,
)
}

func loadManagerOptionsFromFile(path string, options ctrl.Options) (ctrl.Options, error) {
content, err := os.ReadFile(path)
if err != nil {
Expand Down
69 changes: 69 additions & 0 deletions pkg/manager/run_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package manager

import (
"context"
"reflect"
"testing"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func TestNewCredentialClientUsesAPIReaderForSecretReads(t *testing.T) {
t.Parallel()

scheme := runtime.NewScheme()
if err := corev1.AddToScheme(scheme); err != nil {
t.Fatalf("add corev1 scheme: %v", err)
}

secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "credential", Namespace: "default"},
Data: map[string][]byte{"key": []byte("value")},
}
apiReader := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret.DeepCopy()).Build()
cachedClient := &staleSecretGetClient{
Client: apiReader,
getErr: apierrors.NewNotFound(corev1.Resource("secrets"), secret.Name),
}

credClient := newCredentialClient(&fakeCredentialClientManager{
client: cachedClient,
apiReader: apiReader,
}, nil)

data, err := credClient.GetSecret(context.Background(), secret.Name, secret.Namespace)
if err != nil {
t.Fatalf("GetSecret() error = %v", err)
}
if !reflect.DeepEqual(data, secret.Data) {
t.Fatalf("GetSecret() data = %v, want %v", data, secret.Data)
}
if cachedClient.getCalls != 0 {
t.Fatalf("cached client Get calls = %d, want 0", cachedClient.getCalls)
}
}

type staleSecretGetClient struct {
ctrlclient.Client
getCalls int
getErr error
}

func (c *staleSecretGetClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, opts ...ctrlclient.GetOption) error {
c.getCalls++
return c.getErr
}

type fakeCredentialClientManager struct {
client ctrlclient.Client
apiReader ctrlclient.Reader
}

func (m *fakeCredentialClientManager) GetClient() ctrlclient.Client { return m.client }

func (m *fakeCredentialClientManager) GetAPIReader() ctrlclient.Reader { return m.apiReader }
Loading