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

operator install: honor operatorframework.io/suggested-namespace #52

Closed
Closed
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
3 changes: 0 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,6 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J
github.com/onsi/gomega v1.8.1/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA=
github.com/onsi/gomega v1.9.0/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA=
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
github.com/onsi/gomega v1.10.2 h1:aY/nuoWlKJud2J6U0E3NWsjlg+0GtwXxgEqthRdzlcs=
github.com/onsi/gomega v1.10.2/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
github.com/onsi/gomega v1.11.0 h1:+CqWgvj0OZycCaqclBD1pxKHAU+tOkHmQIWvDHq2aug=
github.com/onsi/gomega v1.11.0/go.mod h1:azGKhqFUon9Vuj0YmTfLSmx0FUwqXYSTl5re8lQLTUg=
Expand Down Expand Up @@ -898,7 +897,6 @@ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLL
golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
golang.org/x/net v0.0.0-20201110031124-69a78807bb2b h1:uwuIcX0g4Yl1NC5XAz37xsr2lTtcqevgzYNVt49waME=
golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/net v0.0.0-20201202161906-c7110b5ffcbb h1:eBmm0M9fYhWpKZLjQUUKka/LtIxf46G4fxeEz5KJr9U=
golang.org/x/net v0.0.0-20201202161906-c7110b5ffcbb/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
Expand Down Expand Up @@ -1147,7 +1145,6 @@ gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.7/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU=
gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
Expand Down
7 changes: 7 additions & 0 deletions internal/cmd/operator_install.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"errors"
"fmt"
"time"

Expand All @@ -25,6 +26,11 @@ func newOperatorInstallCmd(cfg *action.Configuration) *cobra.Command {
i.Package = args[0]
csv, err := i.Run(cmd.Context())
if err != nil {
if errors.Is(err, internalaction.ErrNoOperatorGroup) {
log.Fatalf("operator group not found in namespace %q, use --create-operator-group to create one automatically", cfg.Namespace)
} else if altNsErr := (&internalaction.ErrIncorrectNamespace{}); errors.As(err, altNsErr) {
log.Fatalf("invalid installation namespace: use --namespace=%q to install into operator's suggested namespace or --permit-alternate-namespace to force installation in %q", altNsErr.Suggested, altNsErr.Requested)
}
log.Fatalf("failed to install operator: %v", err)
}
log.Printf("operator %q installed; installed csv is %q", i.Package, csv.Name)
Expand All @@ -42,4 +48,5 @@ func bindOperatorInstallFlags(fs *pflag.FlagSet, i *internalaction.OperatorInsta
fs.StringSliceVarP(&i.WatchNamespaces, "watch", "w", []string{}, "namespaces to watch")
fs.DurationVar(&i.CleanupTimeout, "cleanup-timeout", time.Minute, "the amount of time to wait before cancelling cleanup")
fs.BoolVarP(&i.CreateOperatorGroup, "create-operator-group", "C", false, "create operator group if necessary")
fs.BoolVar(&i.PermitAlternateNamespace, "permit-alternate-namespace", false, "permit an alternate namespace to be used when the operator defines operatorframework.io/suggested-namespace")
}
46 changes: 38 additions & 8 deletions internal/pkg/action/operator_install.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package action

import (
"context"
"errors"
"fmt"
"regexp"
"strings"
Expand All @@ -10,6 +11,7 @@ import (
v1 "github.com/operator-framework/api/pkg/operators/v1"
"github.com/operator-framework/api/pkg/operators/v1alpha1"
operatorsv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apis/operators/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
Expand All @@ -23,13 +25,14 @@ import (
type OperatorInstall struct {
config *action.Configuration

Package string
Channel string
Version string
Approval subscription.ApprovalValue
WatchNamespaces []string
CleanupTimeout time.Duration
CreateOperatorGroup bool
Package string
Channel string
Version string
Approval subscription.ApprovalValue
WatchNamespaces []string
CleanupTimeout time.Duration
CreateOperatorGroup bool
PermitAlternateNamespace bool

Logf func(string, ...interface{})
}
Expand All @@ -41,6 +44,17 @@ func NewOperatorInstall(cfg *action.Configuration) *OperatorInstall {
}
}

var ErrNoOperatorGroup = errors.New("operator group not found")

type ErrIncorrectNamespace struct {
Requested string
Suggested string
}

func (e ErrIncorrectNamespace) Error() string {
return fmt.Sprintf("requested install namespace is %q, but operator's suggested namespace is %q", e.Requested, e.Suggested)
}

func (i *OperatorInstall) Run(ctx context.Context) (*v1alpha1.ClusterServiceVersion, error) {
pm, err := i.getPackageManifest(ctx)
if err != nil {
Expand All @@ -52,6 +66,10 @@ func (i *OperatorInstall) Run(ctx context.Context) (*v1alpha1.ClusterServiceVers
return nil, fmt.Errorf("get package channel: %v", err)
}

if err := i.ensureNamespace(ctx, pc); err != nil {
return nil, err
}

if _, err := i.ensureOperatorGroup(ctx, pm, pc); err != nil {
return nil, err
}
Expand Down Expand Up @@ -110,6 +128,18 @@ func (i *OperatorInstall) getPackageManifest(ctx context.Context) (*operator.Pac
return &operator.PackageManifest{PackageManifest: *pm}, nil
}

func (i *OperatorInstall) ensureNamespace(ctx context.Context, pc *operator.PackageChannel) error {
suggestedNamespace := pc.CurrentCSVDesc.Annotations["operatorframework.io/suggested-namespace"]
Copy link
Member

Choose a reason for hiding this comment

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

This could panic I believe.

Copy link
Member Author

Choose a reason for hiding this comment

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

It can panic if pc is nil, but that can't happen because pm.GetChannel() (called one function up in the stack) doesn't return nil, nil

Other than that, it can't panic:

  • pc.CurrentCSVDesc is a concrete struct
  • pc.CurrentCSVDesc.Annotations is a map[string]string

If you do a lookup of a key in a nil map, the returned value is the default value of the value type.

Copy link
Member

Choose a reason for hiding this comment

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

If you do a lookup of a key in a nil map, the returned value is the default value of the value type.

TIL! This is kinda crazy to me because a map is really a pointer to an hmap, so you're dereferencing and indexing a nil pointer.

if !i.PermitAlternateNamespace && suggestedNamespace != "" && i.config.Namespace != suggestedNamespace {
return ErrIncorrectNamespace{Suggested: suggestedNamespace, Requested: i.config.Namespace}
}
ns := corev1.Namespace{}
if err := i.config.Client.Get(ctx, types.NamespacedName{Name: i.config.Namespace}, &ns); err != nil {
return err
}
return nil
}

func (i *OperatorInstall) ensureOperatorGroup(ctx context.Context, pm *operator.PackageManifest, pc *operator.PackageChannel) (*v1.OperatorGroup, error) {
og, err := i.getOperatorGroup(ctx)
if err != nil {
Expand Down Expand Up @@ -144,7 +174,7 @@ func (i *OperatorInstall) ensureOperatorGroup(ctx context.Context, pm *operator.
}
i.Logf("operatorgroup %q created", og.Name)
} else {
return nil, fmt.Errorf("namespace %q has no existing operator group; use --create-operator-group to create one automatically", i.config.Namespace)
return nil, ErrNoOperatorGroup
}
}
return og, nil
Expand Down
2 changes: 2 additions & 0 deletions pkg/action/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/operator-framework/api/pkg/operators/v1alpha1"
operatorsv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apis/operators/v1"
"github.com/spf13/pflag"
corev1 "k8s.io/api/core/v1"
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/clientcmd"
Expand All @@ -16,6 +17,7 @@ import (
func NewScheme() (*runtime.Scheme, error) {
sch := runtime.NewScheme()
for _, f := range []func(*runtime.Scheme) error{
corev1.AddToScheme,
v1alpha1.AddToScheme,
operatorsv1.AddToScheme,
v1.AddToScheme,
Expand Down