Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/operator-sdk/new/cmd.go: fallback to default helm RBAC role without kubeconfig #1627

Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
- `CreateMetricsService()` function from the metrics package accepts a REST config (\*rest.Config) and an array of ServicePort objects ([]v1.ServicePort) as input to create Service metrics. `CRPortName` constant is added to describe the string of custom resource port name. ([#1560](https://github.com/operator-framework/operator-sdk/pull/1560) and [#1626](https://github.com/operator-framework/operator-sdk/pull/1626))
- Changed the flag `--skip-git-init` to [`--git-init`](https://github.com/operator-framework/operator-sdk/blob/master/doc/sdk-cli-reference.md#new). This changes the default behavior of `operator-sdk new` to not initialize the new project directory as a git repository with `git init`. This behavior is now opt-in with `--git-init`. ([#1588](https://github.com/operator-framework/operator-sdk/pull/1588))
- `operator-sdk new` will no longer create the initial commit for a new project, even with `--git-init=true`. ([#1588](https://github.com/operator-framework/operator-sdk/pull/1588))
- When errors occur setting up the Kubernetes client for RBAC role generation, `operator-sdk new --type=helm` now falls back to a default RBAC role instead of failing. ([#1627](https://github.com/operator-framework/operator-sdk/pull/1627))

### Deprecated

Expand Down
22 changes: 8 additions & 14 deletions cmd/operator-sdk/new/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,19 +325,13 @@ func doHelmScaffold() error {
valuesPath := filepath.Join("<project_dir>", helm.HelmChartsDir, chart.GetMetadata().GetName(), "values.yaml")
crSpec := fmt.Sprintf("# Default values copied from %s\n\n%s", valuesPath, chart.GetValues().GetRaw())

k8sCfg, err := config.GetConfig()
if err != nil {
return fmt.Errorf("failed to get kubernetes config: %s", err)
}

dc, err := discovery.NewDiscoveryClientForConfig(k8sCfg)
if err != nil {
return fmt.Errorf("failed to get kubernetes discovery client: %s", err)
}

roleScaffold, err := helm.CreateRoleScaffold(dc, chart)
if err != nil {
return fmt.Errorf("failed to generate role scaffold: %s", err)
roleScaffold := helm.DefaultRoleScaffold
if k8sCfg, err := config.GetConfig(); err != nil {
log.Warnf("Using default RBAC rules: failed to get Kubernetes config: %s", err)
} else if dc, err := discovery.NewDiscoveryClientForConfig(k8sCfg); err != nil {
Copy link
Member

@lilic lilic Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens here if k8sCfg is nil and we pass it to NewDiscoveryClientForConfig?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, config.GetConfig() would have to return nil, nil, which is probably not something that it would ever do. But better safe than sorry 🙂 . I'll check if the follow-on functions handle nil for both k8sCfg and dc.

Copy link
Member

@lilic lilic Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed the else part of the if, nvm. :)

log.Warnf("Using default RBAC rules: failed to create Kubernetes discovery client: %s", err)
} else {
roleScaffold = helm.GenerateRoleScaffold(dc, chart)
}

s := &scaffold.Scaffold{}
Expand All @@ -348,7 +342,7 @@ func doHelmScaffold() error {
ChartName: chart.GetMetadata().GetName(),
},
&scaffold.ServiceAccount{},
roleScaffold,
&roleScaffold,
&scaffold.RoleBinding{IsClusterScoped: roleScaffold.IsClusterScoped},
&helm.Operator{},
&scaffold.CRD{Resource: resource},
Expand Down
52 changes: 27 additions & 25 deletions internal/pkg/scaffold/helm/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,40 +44,42 @@ type roleDiscoveryInterface interface {
ServerResources() ([]*metav1.APIResourceList, error)
}

// CreateRoleScaffold generates a role scaffold from the provided helm chart. It
var DefaultRoleScaffold = scaffold.Role{
IsClusterScoped: false,
SkipDefaultRules: false,
CustomRules: []rbacv1.PolicyRule{
// We need this rule so tiller can read namespaces to ensure they exist
{
APIGroups: []string{""},
Resources: []string{"namespaces"},
Verbs: []string{"get"},
},

// We need this rule for leader election and release state storage to work
{
APIGroups: []string{""},
Resources: []string{"configmaps", "secrets"},
Verbs: []string{rbacv1.VerbAll},
},
},
}

// GenerateRoleScaffold generates a role scaffold from the provided helm chart. It
// renders a release manifest using the chart's default values and uses the Kubernetes
// discovery API to lookup each resource in the resulting manifest.
// The role scaffold will have IsClusterScoped=true if the chart lists cluster scoped resources
func CreateRoleScaffold(dc roleDiscoveryInterface, chart *chart.Chart) (*scaffold.Role, error) {
func GenerateRoleScaffold(dc roleDiscoveryInterface, chart *chart.Chart) scaffold.Role {
log.Info("Generating RBAC rules")

roleScaffold := &scaffold.Role{
IsClusterScoped: false,
SkipDefaultRules: true,
CustomRules: []rbacv1.PolicyRule{
// We need this rule so tiller can read namespaces to ensure they exist
{
APIGroups: []string{""},
Resources: []string{"namespaces"},
Verbs: []string{"get"},
},

// We need this rule for leader election and release state storage to work
{
APIGroups: []string{""},
Resources: []string{"configmaps", "secrets"},
Verbs: []string{rbacv1.VerbAll},
},
},
}

roleScaffold := DefaultRoleScaffold
clusterResourceRules, namespacedResourceRules, err := generateRoleRules(dc, chart)
if err != nil {
log.Warnf("Using default RBAC rules: failed to generate RBAC rules: %s", err)
roleScaffold.SkipDefaultRules = false
return roleScaffold, nil
return roleScaffold
}

roleScaffold.SkipDefaultRules = true

// Use a ClusterRole if cluster scoped resources are listed in the chart
if len(clusterResourceRules) > 0 {
log.Info("Scaffolding ClusterRole and ClusterRolebinding for cluster scoped resources in the helm chart")
Expand All @@ -90,7 +92,7 @@ func CreateRoleScaffold(dc roleDiscoveryInterface, chart *chart.Chart) (*scaffol
" some existing rules may be overly broad. Double check the rules generated in deploy/role.yaml" +
" to ensure they meet the operator's permission requirements.")

return roleScaffold, nil
return roleScaffold
}

func generateRoleRules(dc roleDiscoveryInterface, chart *chart.Chart) ([]rbacv1.PolicyRule, []rbacv1.PolicyRule, error) {
Expand Down
11 changes: 2 additions & 9 deletions internal/pkg/scaffold/helm/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"k8s.io/helm/pkg/proto/hapi/chart"
)

func TestCreateRoleScaffold(t *testing.T) {
func TestGenerateRoleScaffold(t *testing.T) {
dcs := map[string]*mockRoleDiscoveryClient{
"upstream": &mockRoleDiscoveryClient{
serverVersion: func() (*version.Info, error) { return &version.Info{Major: "1", Minor: "11"}, nil },
Expand Down Expand Up @@ -66,13 +66,7 @@ func TestCreateRoleScaffold(t *testing.T) {
for dcName, dc := range dcs {
testName := fmt.Sprintf("%s %s", dcName, tc.name)
t.Run(testName, func(t *testing.T) {
roleScaffold, err := helm.CreateRoleScaffold(dc, tc.chart)
if tc.expectErr {
assert.Error(t, err)
return
} else {
assert.NoError(t, err)
}
roleScaffold := helm.GenerateRoleScaffold(dc, tc.chart)
assert.Equal(t, tc.expectSkipDefaultRules, roleScaffold.SkipDefaultRules)
assert.Equal(t, tc.expectLenCustomRules, len(roleScaffold.CustomRules))
assert.Equal(t, tc.expectIsClusterScoped, roleScaffold.IsClusterScoped)
Expand Down Expand Up @@ -120,7 +114,6 @@ type roleScaffoldTestCase struct {
expectSkipDefaultRules bool
expectIsClusterScoped bool
expectLenCustomRules int
expectErr bool
}

func failChart() *chart.Chart {
Expand Down