Skip to content

Commit

Permalink
internal/generate/{collector/clusterserviceversion}: consider
Browse files Browse the repository at this point in the history 
(cluster) role bindings so CSV generator can assign the correct
service account names to roles
  • Loading branch information
@estroz
estroz committed Jul 30, 2020
1 parent a383aa5 commit 8ae4709
Show file tree
Hide file tree
Showing 48 changed files with 461 additions and 2,555 deletions.
67 changes: 16 additions & 51 deletions internal/generate/clusterserviceversion/clusterserviceversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package clusterserviceversion

import (
"bytes"
"encoding/json"
"fmt"
"io/ioutil"
"os"
Expand All @@ -32,7 +31,6 @@ import (
"github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/operator-framework/operator-registry/pkg/lib/bundle"
appsv1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/kubebuilder/pkg/model/config"
"sigs.k8s.io/yaml"

Expand All @@ -46,16 +44,12 @@ var (
testDataDir = filepath.Join("..", "testdata")
csvDir = filepath.Join(testDataDir, "clusterserviceversions")
csvBasesDir = filepath.Join(csvDir, "bases")
csvNewLayoutBundleDir = filepath.Join(csvDir, "newlayout", "manifests")

// TODO: create a new testdata dir (top level?) that has both a "config"
// dir and a "deploy" dir that contains `kustomize build config/default`
// output to simulate actual manifest collection behavior. Using "config"
// directly is not standard behavior.
goTestDataDir = filepath.Join(testDataDir, "non-standard-layout")
goAPIsDir = filepath.Join(goTestDataDir, "api")
goConfigDir = filepath.Join(goTestDataDir, "config")
goCRDsDir = filepath.Join(goConfigDir, "crds")
csvNewLayoutBundleDir = filepath.Join(csvDir, "output")

goTestDataDir = filepath.Join(testDataDir, "go")
goAPIsDir = filepath.Join(goTestDataDir, "api")
goStaticDir = filepath.Join(goTestDataDir, "static")
goBasicOperatorPath = filepath.Join(goStaticDir, "basic.operatory.yaml")
)

var (
Expand All @@ -75,7 +69,7 @@ var (

var _ = BeforeSuite(func() {
col = &collector.Manifests{}
Expect(col.UpdateFromDirs(goConfigDir, goCRDsDir)).ToNot(HaveOccurred())
collectManifestsFromFileHelper(col, goBasicOperatorPath)

cfg = readConfigHelper(goTestDataDir)

Expand All @@ -98,7 +92,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
buf = &bytes.Buffer{}
})

Describe("for the new Go project layout", func() {
Describe("for a Go project", func() {

Context("with correct Options", func() {

Expand Down Expand Up @@ -282,7 +276,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
getBase: makeBaseGetter(newCSV),
}
// Update the input's and expected CSV's Deployment image.
Expect(g.Collector.UpdateFromDirs(goConfigDir, goCRDsDir)).ToNot(HaveOccurred())
collectManifestsFromFileHelper(g.Collector, goBasicOperatorPath)
Expect(len(g.Collector.Deployments)).To(BeNumerically(">=", 1))
imageTag := "controller:v" + g.Version
modifyDepImageHelper(&g.Collector.Deployments[0].Spec, imageTag)
Expand Down Expand Up @@ -312,42 +306,6 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
Expect(csv).To(Equal(upgradeCSV(newCSV, g.OperatorName, g.Version)))
})
})

Context("generate ClusterServiceVersion", func() {
It("should handle CRDs with core type name", func() {
g = Generator{
OperatorName: operatorName,
OperatorType: operatorType,
Version: version,
Collector: &collector.Manifests{},
config: cfg,
getBase: makeBaseGetter(newCSV),
}
err := filepath.Walk(goConfigDir, func(path string, info os.FileInfo, err error) error {
if err != nil || info.IsDir() {
return err
}
file, err := os.OpenFile(path, os.O_RDONLY, 0)
if err != nil {
return err
}
defer file.Close()
return g.Collector.UpdateFromReader(file)
})
Expect(err).ShouldNot(HaveOccurred(), "failed to read manifests")
Expect(len(g.Collector.V1beta1CustomResourceDefinitions)).Should(BeEquivalentTo(2))
Expect(len(g.Collector.CustomResources)).Should(BeEquivalentTo(2))

csv, err := g.generate()
Expect(err).ToNot(HaveOccurred())
Expect(csv.Annotations["alm-examples"]).ShouldNot(BeEquivalentTo("[]"))

crs := []unstructured.Unstructured{}
err = json.Unmarshal([]byte(csv.Annotations["alm-examples"]), &crs)
Expect(err).ShouldNot(HaveOccurred(), "failed to parse 'alm-examples' annotations")
Expect(crs).Should(ConsistOf(g.Collector.CustomResources), "custom resources shall match with CSV annotations")
})
})
})

})
Expand Down Expand Up @@ -389,6 +347,13 @@ var _ = Describe("Generation requires interaction", func() {
})
})

func collectManifestsFromFileHelper(col *collector.Manifests, path string) {
f, err := os.Open(path)
ExpectWithOffset(1, err).ToNot(HaveOccurred())
ExpectWithOffset(1, col.UpdateFromReader(f)).ToNot(HaveOccurred())
ExpectWithOffset(1, f.Close()).Should(Succeed())
}

func readConfigHelper(dir string) *config.Config {
wd, err := os.Getwd()
ExpectWithOffset(1, err).ToNot(HaveOccurred())
Expand Down
86 changes: 69 additions & 17 deletions internal/generate/clusterserviceversion/clusterserviceversion_updaters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
admissionregv1 "k8s.io/api/admissionregistration/v1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/version"

"github.com/operator-framework/operator-sdk/internal/generate/collector"
Expand All @@ -39,7 +40,7 @@ import (
func ApplyTo(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion) error {
// Apply manifests to the CSV object.
if err := apply(c, csv); err != nil {
return fmt.Errorf("error updating ClusterServiceVersion: %v", err)
return err
}

// Set fields required by namespaced operators. This is a no-op for cluster-
Expand All @@ -65,7 +66,7 @@ func apply(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion)

applyCustomResourceDefinitions(c, csv)
if err := applyCustomResources(c, csv); err != nil {
return fmt.Errorf("error applying Custom Resource: %v", err)
return fmt.Errorf("error applying Custom Resource examples to CSV %s: %v", csv.GetName(), err)
}
applyWebhooks(c, csv)
return nil
Expand All @@ -80,29 +81,80 @@ func getCSVInstallStrategy(csv *operatorsv1alpha1.ClusterServiceVersion) operato
return csv.Spec.InstallStrategy
}

// applyRoles updates strategy's permissions with the Roles in the collector.
// applyRoles applies Roles to strategy's permissions field while respecting ServiceAccount names in RoleBindings.
//nolint:dupl
func applyRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) {
// Collect all role service accounts by their corresponding service accounts via bindings.
// This lets us look up all service accounts a role is bound to and create one set of permissions per service account.
saNamesToRoleNames := make(map[string]map[string]struct{})
for _, binding := range c.RoleBindings {
roleRef := binding.RoleRef
if roleRef.Kind == "Role" && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) {
for _, name := range getSubjectServiceAccountNames(binding.Subjects) {
if saNamesToRoleNames[name] == nil {
saNamesToRoleNames[name] = make(map[string]struct{})
}
saNamesToRoleNames[name][roleRef.Name] = struct{}{}
}
}
}

// Apply relevant roles to each service account.
perms := []operatorsv1alpha1.StrategyDeploymentPermissions{}
for _, role := range c.Roles {
perms = append(perms, operatorsv1alpha1.StrategyDeploymentPermissions{
ServiceAccountName: role.GetName(),
Rules: role.Rules,
})
for saName, roleNames := range saNamesToRoleNames {
p := operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName}
for _, role := range c.Roles {
if _, ok := roleNames[role.GetName()]; ok {
p.Rules = append(p.Rules, role.Rules...)
}
}
perms = append(perms, p)
}
strategy.Permissions = perms
}

// applyClusterRoles updates strategy's cluserPermissions with the ClusterRoles
// in the collector.
// applyClusterRoles applies ClusterRoles to strategy's clusterPermissions field while respecting
// ServiceAccount names in ClusterRoleBindings.
//nolint:dupl
func applyClusterRoles(c *collector.Manifests, strategy *operatorsv1alpha1.StrategyDetailsDeployment) {
perms := []operatorsv1alpha1.StrategyDeploymentPermissions{}
for _, role := range c.ClusterRoles {
perms = append(perms, operatorsv1alpha1.StrategyDeploymentPermissions{
ServiceAccountName: role.GetName(),
Rules: role.Rules,
})
// Collect all cluster role service accounts by their corresponding service accounts via bindings.
// This lets us look up all service accounts a cluster role is bound to and create one set of clusterPermissions
// per service account.
saNamesToClusterRoleNames := make(map[string]map[string]struct{})
for _, binding := range c.ClusterRoleBindings {
roleRef := binding.RoleRef
if roleRef.Kind == "ClusterRole" && (roleRef.APIGroup == "" || roleRef.APIGroup == rbacv1.SchemeGroupVersion.Group) {
for _, name := range getSubjectServiceAccountNames(binding.Subjects) {
if saNamesToClusterRoleNames[name] == nil {
saNamesToClusterRoleNames[name] = make(map[string]struct{})
}
saNamesToClusterRoleNames[name][roleRef.Name] = struct{}{}
}
}
}

// Apply relevant cluster roles to each service account.
clusterPerms := []operatorsv1alpha1.StrategyDeploymentPermissions{}
for saName, clusterRoleNames := range saNamesToClusterRoleNames {
p := operatorsv1alpha1.StrategyDeploymentPermissions{ServiceAccountName: saName}
for _, clusterRole := range c.ClusterRoles {
if _, ok := clusterRoleNames[clusterRole.GetName()]; ok {
p.Rules = append(p.Rules, clusterRole.Rules...)
}
}
clusterPerms = append(clusterPerms, p)
}
strategy.ClusterPermissions = clusterPerms
}

// getSubjectServiceAccountNames returns a list of all ServiceAccount subject names.
func getSubjectServiceAccountNames(subjects []rbacv1.Subject) (saNames []string) {
for _, subject := range subjects {
if subject.Kind == "ServiceAccount" {
saNames = append(saNames, subject.Name)
}
}
strategy.ClusterPermissions = perms
return saNames
}

// applyDeployments updates strategy's deployments with the Deployments
Expand Down
36 changes: 36 additions & 0 deletions internal/generate/collector/collect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import (
type Manifests struct {
Roles []rbacv1.Role
ClusterRoles []rbacv1.ClusterRole
RoleBindings []rbacv1.RoleBinding
ClusterRoleBindings []rbacv1.ClusterRoleBinding
Deployments []appsv1.Deployment
V1CustomResourceDefinitions []apiextv1.CustomResourceDefinition
V1beta1CustomResourceDefinitions []apiextv1beta1.CustomResourceDefinition
Expand All @@ -54,6 +56,8 @@ type Manifests struct {
var (
roleGK = rbacv1.SchemeGroupVersion.WithKind("Role").GroupKind()
clusterRoleGK = rbacv1.SchemeGroupVersion.WithKind("ClusterRole").GroupKind()
roleBindingGK = rbacv1.SchemeGroupVersion.WithKind("RoleBinding").GroupKind()
clusterRoleBindingGK = rbacv1.SchemeGroupVersion.WithKind("ClusterRoleBinding").GroupKind()
deploymentGK = appsv1.SchemeGroupVersion.WithKind("Deployment").GroupKind()
v1crdGK = apiextv1.SchemeGroupVersion.WithKind("CustomResourceDefinition").GroupKind()
v1beta1crdGK = apiextv1beta1.SchemeGroupVersion.WithKind("CustomResourceDefinition").GroupKind()
Expand Down Expand Up @@ -92,6 +96,10 @@ func (c *Manifests) UpdateFromDirs(deployDir, crdsDir string) error {
err = c.addRoles(manifest)
case clusterRoleGK:
err = c.addClusterRoles(manifest)
case roleBindingGK:
err = c.addRoleBindings(manifest)
case clusterRoleBindingGK:
err = c.addClusterRoleBindings(manifest)
case deploymentGK:
err = c.addDeployments(manifest)
case v1crdGK, v1beta1crdGK:
Expand Down Expand Up @@ -154,6 +162,10 @@ func (c *Manifests) UpdateFromReader(r io.Reader) error {
err = c.addRoles(manifest)
case clusterRoleGK:
err = c.addClusterRoles(manifest)
case roleBindingGK:
err = c.addRoleBindings(manifest)
case clusterRoleBindingGK:
err = c.addClusterRoleBindings(manifest)
case deploymentGK:
err = c.addDeployments(manifest)
case v1crdGK, v1beta1crdGK:
Expand Down Expand Up @@ -212,6 +224,30 @@ func (c *Manifests) addClusterRoles(rawManifests ...[]byte) error {
return nil
}

// addRoleBindings assumes all manifest data in rawManifests are RoleBindings and adds them to the collector.
func (c *Manifests) addRoleBindings(rawManifests ...[]byte) error {
for _, rawManifest := range rawManifests {
binding := rbacv1.RoleBinding{}
if err := yaml.Unmarshal(rawManifest, &binding); err != nil {
return err
}
c.RoleBindings = append(c.RoleBindings, binding)
}
return nil
}

// addClusterRoleBindings assumes all manifest data in rawManifests are ClusterRoleBindings and adds them to the collector.
func (c *Manifests) addClusterRoleBindings(rawManifests ...[]byte) error {
for _, rawManifest := range rawManifests {
binding := rbacv1.ClusterRoleBinding{}
if err := yaml.Unmarshal(rawManifest, &binding); err != nil {
return err
}
c.ClusterRoleBindings = append(c.ClusterRoleBindings, binding)
}
return nil
}

// addDeployments assumes all manifest data in rawManifests are Deployments
// and adds them to the collector.
func (c *Manifests) addDeployments(rawManifests ...[]byte) error {
Expand Down
54 changes: 0 additions & 54 deletions internal/generate/collector/collect_test.go
View file

This file was deleted.

0 comments on commit 8ae4709

Please sign in to comment.