Refactor controller initialization (round #3)

pkg/cmd/server/bootstrappolicy/controller_policy.go

@@ -152,6 +152,42 @@ func init() {
 			eventsRule(),
 		},
 	})
+
+	// imagetrigger-controller
+	addControllerRole(rbac.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraImageTriggerControllerServiceAccountName},
+		Rules: []rbac.PolicyRule{
+			rbac.NewRule("list", "watch").Groups(imageGroup, legacyImageGroup).Resources("imagestreams").RuleOrDie(),
+			rbac.NewRule("get", "update").Groups(extensionsGroup).Resources("daemonsets").RuleOrDie(),
+			rbac.NewRule("get", "update").Groups(extensionsGroup, appsGroup).Resources("deployments").RuleOrDie(),
+			rbac.NewRule("get", "update").Groups(appsGroup).Resources("statefulsets").RuleOrDie(),
+			rbac.NewRule("get", "update").Groups(batchGroup).Resources("cronjobs").RuleOrDie(),
+			rbac.NewRule("get", "update").Groups(deployGroup, legacyDeployGroup).Resources("deploymentconfigs").RuleOrDie(),
+			rbac.NewRule("create").Groups(buildGroup, legacyBuildGroup).Resources("buildconfigs/instantiate").RuleOrDie(),
+			eventsRule(),
+		},
+	})
+
+	// service-serving-cert-controller
+	addControllerRole(rbac.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraServiceServingCertServiceAccountName},
+		Rules: []rbac.PolicyRule{
+			rbac.NewRule("list", "watch", "update").Groups(kapiGroup).Resources("services").RuleOrDie(),
+			rbac.NewRule("get", "list", "watch", "create", "update").Groups(kapiGroup).Resources("secrets").RuleOrDie(),
+			eventsRule(),
+		},
+	})
+
+	// image-import-controller
+	addControllerRole(rbac.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraImageImportControllerServiceAccountName},
+		Rules: []rbac.PolicyRule{
+			rbac.NewRule("get", "list", "watch", "create", "update").Groups(imageGroup, legacyImageGroup).Resources("imagestreams").RuleOrDie(),
+			rbac.NewRule("get", "list", "watch", "create", "update", "patch", "delete").Groups(imageGroup, legacyImageGroup).Resources("images").RuleOrDie(),
+			rbac.NewRule("create").Groups(imageGroup, legacyImageGroup).Resources("imagestreamimports").RuleOrDie(),
+			eventsRule(),
+		},
+	})
 }
 
 // ControllerRoles returns the cluster roles used by controllers

pkg/cmd/server/bootstrappolicy/infra_sa_policy.go

@@ -13,27 +13,27 @@ import (
 
 	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
 	authorizationapiv1 "github.com/openshift/origin/pkg/authorization/api/v1"
-	buildapi "github.com/openshift/origin/pkg/build/api"
-	deployapi "github.com/openshift/origin/pkg/deploy/api"
-	imageapi "github.com/openshift/origin/pkg/image/api"
 	templateapi "github.com/openshift/origin/pkg/template/api"
 
 	// we need the conversions registered for our init block
 	_ "github.com/openshift/origin/pkg/authorization/api/install"
 )
 
 const (
-	InfraBuildControllerServiceAccountName                     = "build-controller"
-	InfraImageTriggerControllerServiceAccountName              = "imagetrigger-controller"
-	ImageTriggerControllerRoleName                             = "system:imagetrigger-controller"
-	InfraDeploymentConfigControllerServiceAccountName          = "deploymentconfig-controller"
-	InfraDeploymentTriggerControllerServiceAccountName         = "deployment-trigger-controller"
-	InfraDeployerControllerServiceAccountName                  = "deployer-controller"
+	// The controllers below were converted to new controller initialization and use RBAC
+	// rules:
 	InfraOriginNamespaceServiceAccountName                     = "origin-namespace-controller"
 	InfraServiceAccountControllerServiceAccountName            = "serviceaccount-controller"
 	InfraServiceAccountPullSecretsControllerServiceAccountName = "serviceaccount-pull-secrets-controller"
 	InfraServiceAccountTokensControllerServiceAccountName      = "serviceaccount-tokens-controller"
+	InfraServiceServingCertServiceAccountName                  = "service-serving-cert-controller"
+	InfraBuildControllerServiceAccountName                     = "build-controller"
 	InfraBuildConfigChangeControllerServiceAccountName         = "build-config-change-controller"
+	InfraDeploymentConfigControllerServiceAccountName          = "deploymentconfig-controller"
+	InfraDeploymentTriggerControllerServiceAccountName         = "deployment-trigger-controller"
+	InfraDeployerControllerServiceAccountName                  = "deployer-controller"
+	InfraImageTriggerControllerServiceAccountName              = "image-trigger-controller"
+	InfraImageImportControllerServiceAccountName               = "image-import-controller"
 
 	InfraPersistentVolumeBinderControllerServiceAccountName = "pv-binder-controller"
 	PersistentVolumeBinderControllerRoleName                = "system:pv-binder-controller"
@@ -53,9 +53,6 @@ const (
 	InfraUnidlingControllerServiceAccountName = "unidling-controller"
 	UnidlingControllerRoleName                = "system:unidling-controller"
 
-	ServiceServingCertServiceAccountName = "service-serving-cert-controller"
-	ServiceServingCertControllerRoleName = "system:service-serving-cert-controller"
-
 	InfraServiceIngressIPControllerServiceAccountName = "service-ingress-ip-controller"
 	ServiceIngressIPControllerRoleName                = "system:service-ingress-ip-controller"
 
@@ -145,57 +142,6 @@ func init() {
 	InfraSAs.serviceAccounts = sets.String{}
 	InfraSAs.saToRole = map[string]authorizationapi.ClusterRole{}
 
-	err = InfraSAs.addServiceAccount(
-		InfraImageTriggerControllerServiceAccountName,
-		authorizationapi.ClusterRole{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: ImageTriggerControllerRoleName,
-			},
-			Rules: []authorizationapi.PolicyRule{
-				// List Watch
-				{
-					Verbs:     sets.NewString("list", "watch"),
-					APIGroups: []string{imageapi.GroupName, imageapi.LegacyGroupName},
-					Resources: sets.NewString("imagestreams"),
-				},
-				// Spec update on triggerable resources
-				{
-					Verbs:     sets.NewString("get", "update"),
-					APIGroups: []string{extensionsGroup},
-					Resources: sets.NewString("daemonsets"),
-				},
-				{
-					Verbs:     sets.NewString("get", "update"),
-					APIGroups: []string{extensionsGroup, appsGroup},
-					Resources: sets.NewString("deployments"),
-				},
-				{
-					Verbs:     sets.NewString("get", "update"),
-					APIGroups: []string{appsGroup},
-					Resources: sets.NewString("statefulsets"),
-				},
-				{
-					Verbs:     sets.NewString("get", "update"),
-					APIGroups: []string{batchGroup},
-					Resources: sets.NewString("cronjobs"),
-				},
-				{
-					Verbs:     sets.NewString("get", "update"),
-					APIGroups: []string{deployapi.GroupName, deployapi.LegacyGroupName},
-					Resources: sets.NewString("deploymentconfigs"),
-				},
-				{
-					Verbs:     sets.NewString("create"),
-					APIGroups: []string{buildapi.GroupName, buildapi.LegacyGroupName},
-					Resources: sets.NewString("buildconfigs/instantiate"),
-				},
-			},
-		},
-	)
-	if err != nil {
-		panic(err)
-	}
-
 	err = InfraSAs.addServiceAccount(
 		InfraPersistentVolumeRecyclerControllerServiceAccountName,
 		authorizationapi.ClusterRole{
@@ -528,30 +474,6 @@ func init() {
 		panic(err)
 	}
 
-	err = InfraSAs.addServiceAccount(
-		ServiceServingCertServiceAccountName,
-		authorizationapi.ClusterRole{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: ServiceServingCertControllerRoleName,
-			},
-			Rules: []authorizationapi.PolicyRule{
-				{
-					APIGroups: []string{kapi.GroupName},
-					Verbs:     sets.NewString("list", "watch", "update"),
-					Resources: sets.NewString("services"),
-				},
-				{
-					APIGroups: []string{kapi.GroupName},
-					Verbs:     sets.NewString("get", "list", "watch", "create", "update"),
-					Resources: sets.NewString("secrets"),
-				},
-			},
-		},
-	)
-	if err != nil {
-		panic(err)
-	}
-
 	err = InfraSAs.addServiceAccount(
 		InfraServiceIngressIPControllerServiceAccountName,
 		authorizationapi.ClusterRole{

pkg/cmd/server/origin/controller.go

@@ -3,26 +3,29 @@ package origin
 import (
 	"fmt"
 	"io/ioutil"
+	"time"
+
+	"github.com/golang/glog"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/util/cert"
 	kapi "k8s.io/kubernetes/pkg/api"
 	kubecontroller "k8s.io/kubernetes/pkg/controller"
 	"k8s.io/kubernetes/pkg/serviceaccount"
 
-	"github.com/golang/glog"
+	configapi "github.com/openshift/origin/pkg/cmd/server/api"
 	"github.com/openshift/origin/pkg/cmd/server/crypto"
 	"github.com/openshift/origin/pkg/cmd/server/origin/controller"
 )
 
 // NewOpenShiftControllerPreStartInitializers returns list of initializers for controllers
 // that needed to be run before any other controller is started.
-// Typically this has to done for the serviceaccount-tokens controller as it provides
+// Typically this has to done for the serviceaccount-token controller as it provides
 // tokens to other controllers.
 func (c *MasterConfig) NewOpenShiftControllerPreStartInitializers() (map[string]controller.InitFunc, error) {
 	ret := map[string]controller.InitFunc{}
 
-	saTokens := controller.ServiceAccountTokensControllerOptions{
+	saToken := controller.ServiceAccountTokenControllerOptions{
 		RootClientBuilder: kubecontroller.SimpleControllerClientBuilder{
 			ClientConfig: &c.PrivilegedLoopbackClientConfig,
 		},
@@ -35,17 +38,17 @@ func (c *MasterConfig) NewOpenShiftControllerPreStartInitializers() (map[string]
 
 	var err error
 
-	saTokens.PrivateKey, err = serviceaccount.ReadPrivateKey(c.Options.ServiceAccountConfig.PrivateKeyFile)
+	saToken.PrivateKey, err = serviceaccount.ReadPrivateKey(c.Options.ServiceAccountConfig.PrivateKeyFile)
 	if err != nil {
 		return nil, fmt.Errorf("error reading signing key for Service Account Token Manager: %v", err)
 	}
 
 	if len(c.Options.ServiceAccountConfig.MasterCA) > 0 {
-		saTokens.RootCA, err = ioutil.ReadFile(c.Options.ServiceAccountConfig.MasterCA)
+		saToken.RootCA, err = ioutil.ReadFile(c.Options.ServiceAccountConfig.MasterCA)
 		if err != nil {
 			return nil, fmt.Errorf("error reading master ca file for Service Account Token Manager: %s: %v", c.Options.ServiceAccountConfig.MasterCA, err)
 		}
-		if _, err := cert.ParseCertsPEM(saTokens.RootCA); err != nil {
+		if _, err := cert.ParseCertsPEM(saToken.RootCA); err != nil {
 			return nil, fmt.Errorf("error parsing master ca file for Service Account Token Manager: %s: %v", c.Options.ServiceAccountConfig.MasterCA, err)
 		}
 	}
@@ -63,27 +66,29 @@ func (c *MasterConfig) NewOpenShiftControllerPreStartInitializers() (map[string]
 		// if we have a rootCA bundle add that too.  The rootCA will be used when hitting the default master service, since those are signed
 		// using a different CA by default.  The rootCA's key is more closely guarded than ours and if it is compromised, that power could
 		// be used to change the trusted signers for every pod anyway, so we're already effectively trusting it.
-		if len(saTokens.RootCA) > 0 {
-			saTokens.ServiceServingCA = append(saTokens.ServiceServingCA, saTokens.RootCA...)
-			saTokens.ServiceServingCA = append(saTokens.ServiceServingCA, []byte("\n")...)
+		if len(saToken.RootCA) > 0 {
+			saToken.ServiceServingCA = append(saToken.ServiceServingCA, saToken.RootCA...)
+			saToken.ServiceServingCA = append(saToken.ServiceServingCA, []byte("\n")...)
 		}
-		saTokens.ServiceServingCA = append(saTokens.ServiceServingCA, serviceServingCA...)
+		saToken.ServiceServingCA = append(saToken.ServiceServingCA, serviceServingCA...)
 	}
-	ret["serviceaccount-tokens"] = saTokens.RunController
+	// this matches the upstream name
+	ret["serviceaccount-token"] = saToken.RunController
 
 	return ret, nil
 }
 
 func (c *MasterConfig) NewOpenshiftControllerInitializers() (map[string]controller.InitFunc, error) {
 	ret := map[string]controller.InitFunc{}
 
+	// TODO this overrides an upstream controller, so move this to where we initialize upstream controllers
 	serviceAccount := controller.ServiceAccountControllerOptions{
 		ManagedNames: c.Options.ServiceAccountConfig.ManagedNames,
 	}
 	ret["serviceaccount"] = serviceAccount.RunController
 
-	ret["serviceaccount-pull-secrets"] = controller.RunServiceAccountPullSecretsController
-	ret["origin-namespace"] = controller.RunOriginNamespaceController
+	ret["openshift.io/serviceaccount-pull-secrets"] = controller.RunServiceAccountPullSecretsController
+	ret["openshift.io/origin-namespace"] = controller.RunOriginNamespaceController
 
 	// initialize build controller
 	storageVersion := c.Options.EtcdStorageConfig.OpenShiftStorageVersion
@@ -97,25 +102,51 @@ func (c *MasterConfig) NewOpenshiftControllerInitializers() (map[string]controll
 		AdmissionPluginConfig: c.Options.AdmissionConfig.PluginConfig,
 		Codec: codec,
 	}
-	ret["build"] = buildControllerConfig.RunController
-	ret["build-config-change"] = controller.RunBuildConfigChangeController
+
+	ret["openshift.io/build"] = buildControllerConfig.RunController
+	ret["openshift.io/build-config-change"] = controller.RunBuildConfigChangeController
 
 	// initialize apps.openshift.io controllers
 	vars, err := c.GetOpenShiftClientEnvVars()
 	if err != nil {
 		return nil, err
 	}
 	deployer := controller.DeployerControllerConfig{ImageName: c.ImageFor("deployer"), Codec: codec, ClientEnvVars: vars}
-	ret["deployer"] = deployer.RunController
+	ret["openshift.io/deployer"] = deployer.RunController
 
 	deploymentConfig := controller.DeploymentConfigControllerConfig{Codec: codec}
-	ret["deploymentconfig"] = deploymentConfig.RunController
+	ret["openshift.io/deploymentconfig"] = deploymentConfig.RunController
 
 	deploymentTrigger := controller.DeploymentTriggerControllerConfig{Codec: codec}
-	ret["deploymenttrigger"] = deploymentTrigger.RunController
+	ret["openshift.io/deploymenttrigger"] = deploymentTrigger.RunController
+
+	// initialize other controllers
+	imageTrigger := controller.ImageTriggerControllerConfig{
+		HasBuilderEnabled: c.Options.DisabledFeatures.Has(configapi.FeatureBuilder),
+		// TODO: make these consts in configapi
+		HasDeploymentsEnabled:  c.Options.DisabledFeatures.Has("triggers.image.openshift.io/deployments"),
+		HasDaemonSetsEnabled:   c.Options.DisabledFeatures.Has("triggers.image.openshift.io/daemonsets"),
+		HasStatefulSetsEnabled: c.Options.DisabledFeatures.Has("triggers.image.openshift.io/statefulsets"),
+		HasCronJobsEnabled:     c.Options.DisabledFeatures.Has("triggers.image.openshift.io/cronjobs"),
+	}
+	ret["openshift.io/image-trigger"] = imageTrigger.RunController
+
+	imageImport := controller.ImageImportControllerOptions{
+		MaxScheduledImageImportsPerMinute: c.Options.ImagePolicyConfig.MaxScheduledImageImportsPerMinute,
+		ResyncPeriod:                      10 * time.Minute,
+
+		DisableScheduledImport:                     c.Options.ImagePolicyConfig.DisableScheduledImport,
+		ScheduledImageImportMinimumIntervalSeconds: c.Options.ImagePolicyConfig.ScheduledImageImportMinimumIntervalSeconds,
+	}
+	ret["openshift.io/image-import"] = imageImport.RunController
 
 	templateInstance := controller.TemplateInstanceControllerConfig{}
-	ret["templateinstance"] = templateInstance.RunController
+	ret["openshift.io/templateinstance"] = templateInstance.RunController
+
+	serviceServingCert := controller.ServiceServingCertsControllerOptions{
+		Signer: c.Options.ControllerConfig.ServiceServingCert.Signer,
+	}
+	ret["openshift.io/service-serving-cert"] = serviceServingCert.RunController
 
 	return ret, nil
 }