Skip to content

Commit

Permalink
Merge pull request #44 from gardener/enh/split-honourServiceAccountRef
Browse files Browse the repository at this point in the history
Split honourServiceAccountRef config
  • Loading branch information
petersutter authored Sep 3, 2020
2 parents 298340f + 2f08d4e commit 0b6fe6a
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 13 deletions.
8 changes: 6 additions & 2 deletions api/v1alpha1/terminal_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,15 @@ type ControllerManagerConfiguration struct {
Webhooks ControllerManagerWebhookConfiguration `yaml:"webhooks"`
// Logger defines the configuration of the zap logging module.
Logger ControllerManagerLoggerConfiguration `yaml:"logger"`
// HonourServiceAccountRef defines if the `credentials.serviceAccountRef` property should be honoured.
// HonourServiceAccountRefHostCluster defines if `host.credentials.serviceAccountRef` property should be honoured.
// It is recommended to be set to false for multi-cluster setups, in case pods are refused on the (virtual) cluster where the terminal resources are stored.
// Defaults to true.
// +optional
HonourServiceAccountRef bool `yaml:"honourServiceAccountRef"`
HonourServiceAccountRefHostCluster bool `yaml:"honourServiceAccountRefHostCluster"`
// HonourServiceAccountRefTargetCluster defines if `target.credentials.serviceAccountRef` property should be honoured.
// Defaults to true.
// +optional
HonourServiceAccountRefTargetCluster bool `yaml:"honourServiceAccountRefTargetCluster"`
}

// ControllerManagerLogger defines the configuration of the Zap Logger.
Expand Down
2 changes: 1 addition & 1 deletion config/overlay/multi-cluster/runtime/manager/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ kind: ControllerManagerConfiguration
apiVersion: v1alpha1
logger:
development: false
honourServiceAccountRef: false # recommended to be disabled for multi-cluster overlay
honourServiceAccountRefHostCluster: false # recommended to be disabled for multi-cluster overlay
8 changes: 4 additions & 4 deletions controllers/terminal_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ func (r *TerminalReconciler) handleRequest(req ctrl.Request) (ctrl.Result, error

gardenClientSet := r.ClientSet

hostClientSet, hostClientSetErr := r.NewClientSetFromClusterCredentials(ctx, gardenClientSet, t.Spec.Host.Credentials)
targetClientSet, targetClientSetErr := r.NewClientSetFromClusterCredentials(ctx, gardenClientSet, t.Spec.Target.Credentials)
hostClientSet, hostClientSetErr := NewClientSetFromClusterCredentials(ctx, gardenClientSet, t.Spec.Host.Credentials, r.Config.HonourServiceAccountRefHostCluster)
targetClientSet, targetClientSetErr := NewClientSetFromClusterCredentials(ctx, gardenClientSet, t.Spec.Target.Credentials, r.Config.HonourServiceAccountRefTargetCluster)

if !t.ObjectMeta.DeletionTimestamp.IsZero() {
// The object is being deleted
Expand Down Expand Up @@ -1039,10 +1039,10 @@ func NewClientSet(config *rest.Config, client client.Client, kubernetes kubernet
return &ClientSet{config, client, kubernetes}
}

func (r *TerminalReconciler) NewClientSetFromClusterCredentials(ctx context.Context, cs *ClientSet, credentials extensionsv1alpha1.ClusterCredentials) (*ClientSet, error) {
func NewClientSetFromClusterCredentials(ctx context.Context, cs *ClientSet, credentials extensionsv1alpha1.ClusterCredentials, honourServiceAccountRef bool) (*ClientSet, error) {
if credentials.SecretRef != nil {
return NewClientSetFromSecretRef(ctx, cs, credentials.SecretRef)
} else if r.Config.HonourServiceAccountRef && credentials.ServiceAccountRef != nil {
} else if honourServiceAccountRef && credentials.ServiceAccountRef != nil {
return NewClientSetFromServiceAccountRef(ctx, cs, credentials.ServiceAccountRef)
} else {
return nil, errors.New("no cluster credentials provided")
Expand Down
3 changes: 2 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ func readControllerManagerConfiguration(configFile string) (*v1alpha1.Controller
Logger: v1alpha1.ControllerManagerLoggerConfiguration{
Development: true,
},
HonourServiceAccountRef: true,
HonourServiceAccountRefHostCluster: true,
HonourServiceAccountRefTargetCluster: true,
}

if err := readFile(configFile, &cfg); err != nil {
Expand Down
10 changes: 5 additions & 5 deletions webhooks/validating/terminal_create_update_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,17 +182,17 @@ func validateRequiredContainerFields(container *v1alpha1.Container, fldPath *fie
}

func (h *TerminalValidator) validateRequiredCredentials(t *v1alpha1.Terminal) error {
if err := h.validateRequiredCredential(t.Spec.Target.Credentials, field.NewPath("spec", "target", "credentials")); err != nil {
if err := validateRequiredCredential(t.Spec.Target.Credentials, field.NewPath("spec", "target", "credentials"), h.Config.HonourServiceAccountRefTargetCluster); err != nil {
return err
}

return h.validateRequiredCredential(t.Spec.Host.Credentials, field.NewPath("spec", "host", "credentials"))
return validateRequiredCredential(t.Spec.Host.Credentials, field.NewPath("spec", "host", "credentials"), h.Config.HonourServiceAccountRefHostCluster)
}

func (h *TerminalValidator) validateRequiredCredential(cred v1alpha1.ClusterCredentials, fldPath *field.Path) error {
if !h.Config.HonourServiceAccountRef {
func validateRequiredCredential(cred v1alpha1.ClusterCredentials, fldPath *field.Path, honourServiceAccountRef bool) error {
if !honourServiceAccountRef {
if cred.ServiceAccountRef != nil {
return field.Forbidden(fldPath, "field is forbidden by configuration")
return field.Forbidden(fldPath.Child("serviceAccountRef"), "field is forbidden by configuration")
}

if cred.SecretRef == nil {
Expand Down

0 comments on commit 0b6fe6a

Please sign in to comment.