Merge pull request #1082 from rancher/refactoring-1079

Refactoring for Readability

.golangci.json

@@ -27,12 +27,9 @@
     "disable-all": true,
     "enable": [
       "misspell",
-      "structcheck",
       "govet",
       "staticcheck",
-      "deadcode",
       "errcheck",
-      "varcheck",
       "unparam",
       "ineffassign",
       "nakedret",

cmd/fleetagent/main.go

@@ -31,7 +31,7 @@ type FleetAgent struct {
 
 func (a *FleetAgent) Run(cmd *cobra.Command, args []string) error {
 	go func() {
-		log.Println(http.ListenAndServe("localhost:6060", nil))
+		log.Println(http.ListenAndServe("localhost:6060", nil)) // nolint:gosec // Debugging only
 	}()
 
 	debugConfig.MustSetupDebug()

cmd/fleetcontroller/main.go

@@ -29,7 +29,7 @@ type FleetManager struct {
 
 func (f *FleetManager) Run(cmd *cobra.Command, args []string) error {
 	go func() {
-		log.Println(http.ListenAndServe("localhost:6060", nil))
+		log.Println(http.ListenAndServe("localhost:6060", nil)) // nolint:gosec // Debugging only
 	}()
 	debugConfig.MustSetupDebug()
 	if err := fleetcontroller.Start(cmd.Context(), f.Namespace, f.Kubeconfig, f.DisableGitops); err != nil {

dev/build-fleet

@@ -8,7 +8,6 @@ if [ ! -d ./cmd/fleetcontroller ]; then
   exit 1
 fi
 
-export GOOS=linux
 export GOARCH="${GOARCH:-amd64}"
 export CGO_ENABLED=0
 
@@ -17,6 +16,7 @@ if ! git diff --quiet HEAD origin/master --  pkg/apis/fleet.cattle.io/v1alpha1;
   go generate
 fi
 
+export GOOS=linux
 # fleet
 go build -gcflags='all=-N -l' -o bin/fleetcontroller-linux-"$GOARCH" ./cmd/fleetcontroller
 docker build -f package/Dockerfile -t rancher/fleet:dev --build-arg="ARCH=$GOARCH"  .

modules/agent/pkg/deployer/manager.go

@@ -121,7 +121,8 @@ func (m *Manager) Resources(bd *fleet.BundleDeployment) (*helmdeployer.Resources
 	return resources, nil
 }
 
-// Deploy the bundle deployment, i.e. with helmdeployer
+// Deploy the bundle deployment, i.e. with helmdeployer.
+// This loads the manifest and the contents from the upstream cluster.
 func (m *Manager) Deploy(bd *fleet.BundleDeployment) (string, error) {
 	if bd.Spec.DeploymentID == bd.Status.AppliedDeploymentID {
 		if ok, err := m.deployer.EnsureInstalled(bd.Name, bd.Status.Release); err != nil {

modules/agent/pkg/register/register.go

@@ -31,7 +31,6 @@ import (
 
 const (
 	CredName            = "fleet-agent"
-	BootstrapCredName   = "fleet-agent-bootstrap" // same as config.AgentBootstrapConfigName for fleet-controller
 	Kubeconfig          = "kubeconfig"
 	Token               = "token"
 	Values              = "values"
@@ -65,10 +64,10 @@ func Register(ctx context.Context, namespace, clusterID string, config *rest.Con
 
 // tryRegister makes sure the secret cattle-fleet-system/fleet-agent is
 // populated and the contained kubeconfig is working
-func tryRegister(ctx context.Context, namespace, clusterID string, config *rest.Config) (*AgentInfo, error) {
-	config = rest.CopyConfig(config)
-	config.RateLimiter = ratelimit.None
-	k8s, err := core.NewFactoryFromConfig(config)
+func tryRegister(ctx context.Context, namespace, clusterID string, cfg *rest.Config) (*AgentInfo, error) {
+	cfg = rest.CopyConfig(cfg)
+	cfg.RateLimiter = ratelimit.None
+	k8s, err := core.NewFactoryFromConfig(cfg)
 	if err != nil {
 		return nil, err
 	}
@@ -78,7 +77,7 @@ func tryRegister(ctx context.Context, namespace, clusterID string, config *rest.
 		// fallback to local cattle-fleet-system/fleet-agent-bootstrap
 		secret, err = runRegistration(ctx, k8s.Core().V1(), namespace, clusterID)
 		if err != nil {
-			return nil, fmt.Errorf("looking up secret %s/%s: %w", namespace, BootstrapCredName, err)
+			return nil, fmt.Errorf("looking up secret %s/%s: %w", namespace, config.AgentBootstrapConfigName, err)
 		}
 	} else if err != nil {
 		return nil, err
@@ -87,7 +86,7 @@ func tryRegister(ctx context.Context, namespace, clusterID string, config *rest.
 		logrus.Errorf("Current credential failed, failing back to reregistering: %v", err)
 		secret, err = runRegistration(ctx, k8s.Core().V1(), namespace, clusterID)
 		if err != nil {
-			return nil, fmt.Errorf("looking up secret %s/%s or %s/%s: %w", namespace, BootstrapCredName, namespace, CredName, err)
+			return nil, fmt.Errorf("looking up secret %s/%s or %s/%s: %w", namespace, config.AgentBootstrapConfigName, namespace, CredName, err)
 		}
 	}
 
@@ -97,7 +96,7 @@ func tryRegister(ctx context.Context, namespace, clusterID string, config *rest.
 	}
 
 	// delete the bootstrap cred
-	_ = k8s.Core().V1().Secret().Delete(namespace, BootstrapCredName, nil)
+	_ = k8s.Core().V1().Secret().Delete(namespace, config.AgentBootstrapConfigName, nil)
 	return &AgentInfo{
 		ClusterNamespace: string(secret.Data[ClusterNamespace]),
 		ClusterName:      string(secret.Data[ClusterName]),
@@ -107,9 +106,9 @@ func tryRegister(ctx context.Context, namespace, clusterID string, config *rest.
 
 func runRegistration(ctx context.Context, k8s corecontrollers.Interface, namespace, clusterID string) (*corev1.Secret, error) {
 	// read cattle-fleet-system/fleet-agent-bootstrap
-	secret, err := k8s.Secret().Get(namespace, BootstrapCredName, metav1.GetOptions{})
+	secret, err := k8s.Secret().Get(namespace, config.AgentBootstrapConfigName, metav1.GetOptions{})
 	if err != nil {
-		return nil, fmt.Errorf("looking up secret %s/%s: %w", namespace, BootstrapCredName, err)
+		return nil, fmt.Errorf("looking up secret %s/%s: %w", namespace, config.AgentBootstrapConfigName, err)
 	}
 	return createClusterSecret(ctx, clusterID, k8s, secret)
 }

modules/agent/pkg/simulator/simulator.go

@@ -103,7 +103,7 @@ func setupNamespace(ctx context.Context, kubeConfig, namespace, simNamespace str
 	clusterID := name.SafeConcatName(simNamespace, strings.SplitN(string(kubeSystem.UID), "-", 2)[0])
 
 	if _, err = k8s.CoreV1().Secrets(simNamespace).Get(ctx, register.CredName, metav1.GetOptions{}); err != nil {
-		secret, err := k8s.CoreV1().Secrets(namespace).Get(ctx, register.BootstrapCredName, metav1.GetOptions{})
+		secret, err := k8s.CoreV1().Secrets(namespace).Get(ctx, config.AgentBootstrapConfigName, metav1.GetOptions{})
 		if err != nil {
 			return "", err
 		}

modules/cli/apply/apply.go

@@ -16,8 +16,8 @@ import (
 
 	"github.com/rancher/fleet/modules/cli/pkg/client"
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
-	"github.com/rancher/fleet/pkg/bundle"
-	"github.com/rancher/fleet/pkg/bundleyaml"
+	"github.com/rancher/fleet/pkg/bundlereader"
+	"github.com/rancher/fleet/pkg/fleetyaml"
 
 	name2 "github.com/rancher/wrangler/pkg/name"
 	"github.com/rancher/wrangler/pkg/yaml"
@@ -47,7 +47,7 @@ type Options struct {
 	Paused          bool
 	Labels          map[string]string
 	SyncGeneration  int64
-	Auth            bundle.Auth
+	Auth            bundlereader.Auth
 }
 
 func globDirs(baseDir string) (result []string, err error) {
@@ -97,7 +97,7 @@ func Apply(ctx context.Context, client *client.Getter, repoName string, baseDirs
 					if !info.IsDir() {
 						return nil
 					}
-					if !bundleyaml.FoundFleetYamlInDirectory(path) {
+					if !fleetyaml.FoundFleetYamlInDirectory(path) {
 						return nil
 					}
 				}
@@ -155,18 +155,18 @@ func pruneBundlesNotFoundInRepo(client *client.Getter, repoName string, gitRepoB
 	return err
 }
 
-// readBundle reads bundle data from a source and return a bundle with the
+// readBundle reads bundle data from a source and returns a bundle with the
 // given name, or the name from the raw source file
-func readBundle(ctx context.Context, name, baseDir string, opts *Options) (*bundle.Bundle, error) {
+func readBundle(ctx context.Context, name, baseDir string, opts *Options) (*fleet.Bundle, []*fleet.ImageScan, error) {
 	if opts.BundleReader != nil {
-		var bundleResource fleet.Bundle
-		if err := json.NewDecoder(opts.BundleReader).Decode(&bundleResource); err != nil {
-			return nil, err
+		var bundle *fleet.Bundle
+		if err := json.NewDecoder(opts.BundleReader).Decode(bundle); err != nil {
+			return nil, nil, err
 		}
-		return bundle.New(&bundleResource)
+		return bundle, nil, nil
 	}
 
-	return bundle.Open(ctx, name, baseDir, opts.BundleFile, &bundle.Options{
+	return bundlereader.Open(ctx, name, baseDir, opts.BundleFile, &bundlereader.Options{
 		Compress:        opts.Compress,
 		Labels:          opts.Labels,
 		ServiceAccount:  opts.ServiceAccount,
@@ -228,12 +228,12 @@ func Dir(ctx context.Context, client *client.Getter, name, baseDir string, opts
 	if opts == nil {
 		opts = &Options{}
 	}
-	bundle, err := readBundle(ctx, createName(name, baseDir), baseDir, opts)
+	bundle, scans, err := readBundle(ctx, createName(name, baseDir), baseDir, opts)
 	if err != nil {
 		return err
 	}
 
-	def := bundle.Definition.DeepCopy()
+	def := bundle.DeepCopy()
 	def.Namespace = client.Namespace
 
 	if len(def.Spec.Resources) == 0 {
@@ -242,7 +242,7 @@ func Dir(ctx context.Context, client *client.Getter, name, baseDir string, opts
 	gitRepoBundlesMap[def.Name] = true
 
 	objects := []runtime.Object{def}
-	for _, scan := range bundle.Scans {
+	for _, scan := range scans {
 		objects = append(objects, scan)
 	}
 
@@ -252,7 +252,7 @@ func Dir(ctx context.Context, client *client.Getter, name, baseDir string, opts
 	}
 
 	if opts.Output == nil {
-		err = save(client, def, bundle.Scans...)
+		err = save(client, def, scans...)
 	} else {
 		_, err = opts.Output.Write(b)
 	}

modules/cli/match/match.go

@@ -1,4 +1,7 @@
 // Package match is used to test matching a bundles to a target on the command line. (fleetapply)
+//
+// It's not used by fleet, but it is available in the fleet CLI as "test" sub
+// command. The tests in fleet-examples use it.
 package match
 
 import (
@@ -10,7 +13,8 @@ import (
 	"os"
 
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
-	"github.com/rancher/fleet/pkg/bundle"
+	"github.com/rancher/fleet/pkg/bundlematcher"
+	"github.com/rancher/fleet/pkg/bundlereader"
 	"github.com/rancher/fleet/pkg/helmdeployer"
 	"github.com/rancher/fleet/pkg/manifest"
 	"github.com/rancher/fleet/pkg/options"
@@ -36,12 +40,12 @@ func Match(ctx context.Context, opts *Options) error {
 	}
 
 	var (
-		b   *bundle.Bundle
-		err error
+		bundle *fleet.Bundle
+		err    error
 	)
 
 	if opts.BundleFile == "" {
-		b, err = bundle.Open(ctx, "test", opts.BaseDir, opts.BundleSpec, nil)
+		bundle, _, err = bundlereader.Open(ctx, "test", opts.BaseDir, opts.BundleSpec, nil)
 		if err != nil {
 			return err
 		}
@@ -51,44 +55,44 @@ func Match(ctx context.Context, opts *Options) error {
 			return err
 		}
 
-		bundleConfig := &fleet.Bundle{}
-		if err := yaml.Unmarshal(data, bundleConfig); err != nil {
+		bundle = &fleet.Bundle{}
+		if err := yaml.Unmarshal(data, bundle); err != nil {
 			return err
 		}
+	}
 
-		b, err = bundle.New(bundleConfig)
-		if err != nil {
-			return err
-		}
+	bm, err := bundlematcher.New(bundle)
+	if err != nil {
+		return err
 	}
 
 	if opts.Target == "" {
-		m := b.Match(opts.ClusterName, map[string]map[string]string{
+		m := bm.Match(opts.ClusterName, map[string]map[string]string{
 			opts.ClusterGroup: opts.ClusterGroupLabels,
 		}, opts.ClusterLabels)
-		return printMatch(b, m, opts.Output)
+		return printMatch(bundle, m, opts.Output)
 	}
 
-	return printMatch(b, b.MatchForTarget(opts.Target), opts.Output)
+	return printMatch(bundle, bm.MatchForTarget(opts.Target), opts.Output)
 }
 
-func printMatch(bundle *bundle.Bundle, m *bundle.Match, output io.Writer) error {
-	if m == nil {
+func printMatch(bundle *fleet.Bundle, target *fleet.BundleTarget, output io.Writer) error {
+	if target == nil {
 		return errors.New("no match found")
 	}
-	fmt.Fprintf(os.Stderr, "# Matched: %s\n", m.Target.Name)
+	fmt.Fprintf(os.Stderr, "# Matched: %s\n", target.Name)
 	if output == nil {
 		return nil
 	}
 
-	opts := options.Calculate(&bundle.Definition.Spec, m.Target)
+	opts := options.Merge(bundle.Spec.BundleDeploymentOptions, target.BundleDeploymentOptions)
 
-	manifest, err := manifest.New(&bundle.Definition.Spec)
+	manifest, err := manifest.New(bundle.Spec.Resources)
 	if err != nil {
 		return err
 	}
 
-	objs, err := helmdeployer.Template(m.Bundle.Definition.Name, manifest, opts)
+	objs, err := helmdeployer.Template(bundle.Name, manifest, opts)
 	if err != nil {
 		return err
 	}

pkg/agent/agent.go

@@ -147,7 +147,7 @@ func getToken(ctx context.Context, controllerNamespace, tokenName string, client
 		return nil, fmt.Errorf("failed to find token in values")
 	}
 
-	expectedNamespace := fleetns.RegistrationNamespace(controllerNamespace)
+	expectedNamespace := fleetns.SystemRegistrationNamespace(controllerNamespace)
 	actualNamespace := data["systemRegistrationNamespace"]
 	if actualNamespace != expectedNamespace {
 		return nil, fmt.Errorf("registration namespace (%s) from secret (%s/%s) does not match expected: %s", actualNamespace, secret.Namespace, secret.Name, expectedNamespace)

pkg/agent/config.go

@@ -4,9 +4,10 @@ import (
 	"context"
 
 	"github.com/rancher/fleet/modules/cli/pkg/client"
-	"github.com/rancher/fleet/pkg/basic"
 	"github.com/rancher/fleet/pkg/config"
 
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
@@ -44,7 +45,11 @@ func configObjects(controllerNamespace string, clusterLabels map[string]string,
 	}
 	cm.Name = "fleet-agent"
 	return []runtime.Object{
-		basic.Namespace(controllerNamespace),
+		&corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: controllerNamespace,
+			},
+		},
 		cm,
 	}, nil
 }