Cleanup controller gen and proto gen

Controller gen has an opinionated directory structure that doesn't
typically compliment Go guidelines on project structure. This change
tidies controller sources used for code gen to better align with Go
project structure best practice.

Additionally, the proto files that are generated and only used
internally have been moved to `internal`.

Signed-off-by: Chris Doherty <[email protected]>

.golangci.yml

@@ -174,8 +174,8 @@ issues:
       linters:
         - noctx
     # local to tink: kubebuilder needs the stdlib invalid `inline` json struct tag
-    - path: pkg/apis/.*
-      text: "struct-tag"
+    - path: api/.*
+      text: "struct-tag: unknown option 'inline'"
     - path: main\.go
       linters:
         - noctx

Makefile

@@ -95,26 +95,26 @@ test: ## Run tests
 	source <($(SETUP_ENVTEST) use -p env) && $(GO) test -coverprofile=coverage.txt ./...
 
 .PHONY: generate-proto
-generate-proto: buf.gen.yaml buf.lock $(shell git ls-files 'protos/*/*.proto') _protoc
+generate-proto: buf.gen.yaml buf.lock $(shell git ls-files '**/*.proto') _protoc
 	$(BUF) mod update
 	$(BUF) generate
-	$(GOFUMPT) -w protos/*/*.pb.*
+	$(GOFUMPT) -w internal/proto/*.pb.*
 
 .PHONY: generate
 generate: generate-proto generate-go generate-manifests ## Generate code, manifests etc.
 
 .PHONY: generate-go
 generate-go:
-	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate/boilerplate.generatego.txt" paths="./pkg/apis/..."
-	$(GOFUMPT) -w ./pkg/apis
+	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate/boilerplate.generatego.txt" paths="./api/..."
+	$(GOFUMPT) -w ./api
 
 .PHONY: generate-manifests
 generate-manifests: generate-crds generate-rbacs generate-server-rbacs ## Generate manifests e.g. CRD, RBAC etc.
 
 .PHONY: generate-crds
 generate-crds:
 	$(CONTROLLER_GEN) \
-		paths=./pkg/apis/... \
+		paths=./api/... \
 		crd:crdVersions=v1 \
 		rbac:roleName=manager-role \
 		output:crd:dir=./config/crd/bases \
@@ -123,17 +123,17 @@ generate-crds:
 	$(YAMLFMT) ./config/crd/bases/* ./config/webhook/*
 
 .PHONY: generate-rbacs
-generate-rbacs: 
+generate-rbacs:
 	$(CONTROLLER_GEN) \
-		paths=./pkg/controllers/... \
+		paths=./internal/controller/... \
 		output:rbac:dir=./config/rbac/ \
 		rbac:roleName=manager-role
 	$(YAMLFMT) ./config/rbac/*
 
 .PHONY: generate-server-rbacs
-generate-server-rbacs: 
+generate-server-rbacs:
 	$(CONTROLLER_GEN) \
-		paths=./server/... \
+		paths=./internal/server/... \
 		output:rbac:dir=./config/server-rbac \
 		rbac:roleName=server-role
 	$(YAMLFMT) ./config/server-rbac/*
@@ -168,9 +168,9 @@ check-generated: check-proto ## Check if generated files are up to date.
 
 .PHONY: check-proto
 check-proto: generate-proto
-	@git diff --no-ext-diff --quiet --exit-code -- protos/*/*.pb.* || (
+	@git diff --no-ext-diff --quiet --exit-code -- **/*.pb.* || (
 	  echo "Protobuf files need to be regenerated!";
-	  git diff --no-ext-diff --exit-code -- protos/*/*.pb.*
+	  git diff --no-ext-diff --exit-code -- **/*.pb.*
 	)
 
 .PHONY: verify

ci-checks.sh

@@ -7,12 +7,12 @@ set -eux
 failed=0
 
 # spell-checks only language files to avoid spell-checking checksums
-if ! git ls-files '*.sh' '*.go' '*.md' | xargs codespell -q 3 -I .codespell-whitelist; then
+if ! git ls-files '*.sh' '*.go' | xargs codespell -q 3 -I .codespell-whitelist; then
 	failed=1
 fi
 
 # --check doesn't show what line number fails, so write the result to disk for the diff to catch
-if ! git ls-files '*.json' '*.md' | xargs prettier --list-different --write; then
+if ! git ls-files '*.json' | xargs prettier --list-different --write; then
 	failed=1
 fi
 

cmd/tink-controller/main.go

@@ -10,8 +10,8 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
-	"github.com/tinkerbell/tink/pkg/controllers"
-	wfctrl "github.com/tinkerbell/tink/pkg/controllers/workflow"
+	"github.com/tinkerbell/tink/internal/controller"
+	"github.com/tinkerbell/tink/internal/workflow"
 	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 )
@@ -72,16 +72,16 @@ func NewRootCommand(config *DaemonConfig, logger log.Logger) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			options := controllers.GetControllerOptions()
+			options := controller.GetControllerOptions()
 			options.LeaderElectionNamespace = namespace
-			manager, err := controllers.NewManager(cfg, options)
+			manager, err := controller.NewManager(cfg, options)
 			if err != nil {
 				return err
 			}
 
 			return manager.RegisterControllers(
 				cmd.Context(),
-				wfctrl.NewController(manager.GetClient()),
+				workflow.NewController(manager.GetClient()),
 			).Start(cmd.Context())
 		},
 	}

cmd/tink-worker/cmd/root.go

@@ -13,7 +13,7 @@ import (
 	"github.com/spf13/viper"
 	"github.com/tinkerbell/tink/cmd/tink-worker/worker"
 	"github.com/tinkerbell/tink/internal/client"
-	"github.com/tinkerbell/tink/protos/workflow"
+	"github.com/tinkerbell/tink/internal/proto"
 )
 
 const (
@@ -51,7 +51,7 @@ func NewRootCommand(version string, logger log.Logger) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			workflowClient := workflow.NewWorkflowServiceClient(conn)
+			workflowClient := proto.NewWorkflowServiceClient(conn)
 
 			dockerClient, err := dockercli.NewClientWithOpts(dockercli.FromEnv, dockercli.WithAPIVersionNegotiation())
 			if err != nil {

cmd/tink-worker/worker/container_manager.go

@@ -11,7 +11,7 @@ import (
 	"github.com/docker/docker/client"
 	"github.com/packethost/pkg/log"
 	"github.com/pkg/errors"
-	pb "github.com/tinkerbell/tink/protos/workflow"
+	"github.com/tinkerbell/tink/internal/proto"
 )
 
 const (
@@ -49,7 +49,7 @@ func NewContainerManager(logger log.Logger, cli DockerClient, registryDetails Re
 	return &containerManager{logger, cli, registryDetails}
 }
 
-func (m *containerManager) CreateContainer(ctx context.Context, cmd []string, wfID string, action *pb.WorkflowAction, captureLogs, privileged bool) (string, error) {
+func (m *containerManager) CreateContainer(ctx context.Context, cmd []string, wfID string, action *proto.WorkflowAction, captureLogs, privileged bool) (string, error) {
 	l := m.getLogger(ctx)
 	config := &container.Config{
 		Image:        path.Join(m.registryDetails.Registry, action.GetImage()),
@@ -98,10 +98,10 @@ func (m *containerManager) StartContainer(ctx context.Context, id string) error
 	return errors.Wrap(m.cli.ContainerStart(ctx, id, types.ContainerStartOptions{}), "DOCKER START")
 }
 
-func (m *containerManager) WaitForContainer(ctx context.Context, id string) (pb.State, error) {
+func (m *containerManager) WaitForContainer(ctx context.Context, id string) (proto.State, error) {
 	// Inspect whether the container is in running state
 	if _, err := m.cli.ContainerInspect(ctx, id); err != nil {
-		return pb.State_STATE_FAILED, nil //nolint:nilerr // error is not nil, but it returns nil
+		return proto.State_STATE_FAILED, nil //nolint:nilerr // error is not nil, but it returns nil
 	}
 
 	// send API call to wait for the container completion
@@ -110,34 +110,34 @@ func (m *containerManager) WaitForContainer(ctx context.Context, id string) (pb.
 	select {
 	case status := <-wait:
 		if status.StatusCode == 0 {
-			return pb.State_STATE_SUCCESS, nil
+			return proto.State_STATE_SUCCESS, nil
 		}
-		return pb.State_STATE_FAILED, nil
+		return proto.State_STATE_FAILED, nil
 	case err := <-errC:
-		return pb.State_STATE_FAILED, err
+		return proto.State_STATE_FAILED, err
 	case <-ctx.Done():
-		return pb.State_STATE_TIMEOUT, ctx.Err()
+		return proto.State_STATE_TIMEOUT, ctx.Err()
 	}
 }
 
-func (m *containerManager) WaitForFailedContainer(ctx context.Context, id string, failedActionStatus chan pb.State) {
+func (m *containerManager) WaitForFailedContainer(ctx context.Context, id string, failedActionStatus chan proto.State) {
 	l := m.getLogger(ctx)
 	// send API call to wait for the container completion
 	wait, errC := m.cli.ContainerWait(ctx, id, container.WaitConditionNotRunning)
 
 	select {
 	case status := <-wait:
 		if status.StatusCode == 0 {
-			failedActionStatus <- pb.State_STATE_SUCCESS
+			failedActionStatus <- proto.State_STATE_SUCCESS
 			return
 		}
-		failedActionStatus <- pb.State_STATE_FAILED
+		failedActionStatus <- proto.State_STATE_FAILED
 	case err := <-errC:
 		l.Error(err)
-		failedActionStatus <- pb.State_STATE_FAILED
+		failedActionStatus <- proto.State_STATE_FAILED
 	case <-ctx.Done():
 		l.Error(ctx.Err())
-		failedActionStatus <- pb.State_STATE_TIMEOUT
+		failedActionStatus <- proto.State_STATE_TIMEOUT
 	}
 }
 

cmd/tink-worker/worker/container_manager_test.go

@@ -12,7 +12,7 @@ import (
 	"github.com/docker/docker/client"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/packethost/pkg/log"
-	pb "github.com/tinkerbell/tink/protos/workflow"
+	"github.com/tinkerbell/tink/internal/proto"
 )
 
 type fakeDockerClient struct {
@@ -91,7 +91,7 @@ func TestContainerManagerCreate(t *testing.T) {
 	cases := []struct {
 		name         string
 		workflowName string
-		action       *pb.WorkflowAction
+		action       *proto.WorkflowAction
 		containerID  string
 		registry     string
 		clientErr    error
@@ -100,7 +100,7 @@ func TestContainerManagerCreate(t *testing.T) {
 		{
 			name:         "Happy Path",
 			workflowName: "saveTheRebelBase",
-			action: &pb.WorkflowAction{
+			action: &proto.WorkflowAction{
 				TaskName:    "UseTheForce",
 				Name:        "blow up the death star",
 				Image:       "yav.in/4/forestmoon",
@@ -114,7 +114,7 @@ func TestContainerManagerCreate(t *testing.T) {
 		{
 			name:         "create failure",
 			workflowName: "saveTheRebelBase",
-			action: &pb.WorkflowAction{
+			action: &proto.WorkflowAction{
 				TaskName:    "UseTheForce",
 				Name:        "blow up the death star",
 				Image:       "yav.in/4/forestmoon",
@@ -206,41 +206,41 @@ func TestContainerManagerWait(t *testing.T) {
 		contextTimeout time.Duration
 		clientErr      error
 		waitErr        error
-		wantState      pb.State
+		wantState      proto.State
 		wantErr        error
 	}{
 		{
 			name:           "Happy Path",
 			containerID:    "nomedalforchewie",
 			dockerResponse: 0,
-			wantState:      pb.State_STATE_SUCCESS,
+			wantState:      proto.State_STATE_SUCCESS,
 		},
 		{
 			name:           "start failure",
 			containerID:    "chewieDied",
 			dockerResponse: 1,
-			wantState:      pb.State_STATE_FAILED,
+			wantState:      proto.State_STATE_FAILED,
 			waitErr:        nil,
 		},
 		{
 			name:           "client wait failure",
 			containerID:    "nomedalforchewie",
 			dockerResponse: 1,
-			wantState:      pb.State_STATE_FAILED,
+			wantState:      proto.State_STATE_FAILED,
 			waitErr:        errors.New("Vader Won"),
 			wantErr:        errors.New("Vader Won"),
 		},
 		{
 			name:        "client inspect failure",
 			containerID: "nomedalforchewie",
-			wantState:   pb.State_STATE_FAILED,
+			wantState:   proto.State_STATE_FAILED,
 			clientErr:   errors.New("inspect failed"),
 			wantErr:     nil,
 		},
 		{
 			name:           "client timeout",
 			containerID:    "nomedalforchewie",
-			wantState:      pb.State_STATE_TIMEOUT,
+			wantState:      proto.State_STATE_TIMEOUT,
 			contextTimeout: time.Millisecond * 2,
 			waitErr:        errors.New("Vader Won"),
 			wantErr:        errors.New("context deadline exceeded"),
@@ -285,33 +285,33 @@ func TestContainerManagerWaitFailed(t *testing.T) {
 		contextTimeout time.Duration
 		waitTime       time.Duration
 		clientErr      error
-		wantState      pb.State
+		wantState      proto.State
 	}{
 		{
 			name:           "Happy Path",
 			containerID:    "nomedalforchewie",
 			dockerResponse: 0,
 			waitTime:       0,
-			wantState:      pb.State_STATE_SUCCESS,
+			wantState:      proto.State_STATE_SUCCESS,
 		},
 		{
 			name:           "start failure",
 			containerID:    "chewieDied",
 			dockerResponse: 1,
-			wantState:      pb.State_STATE_FAILED,
+			wantState:      proto.State_STATE_FAILED,
 			clientErr:      nil,
 		},
 		{
 			name:           "client wait failure",
 			containerID:    "nomedalforchewie",
 			dockerResponse: 1,
-			wantState:      pb.State_STATE_FAILED,
+			wantState:      proto.State_STATE_FAILED,
 			clientErr:      errors.New("Vader Won"),
 		},
 		{
 			name:           "client timeout",
 			containerID:    "nomedalforchewie",
-			wantState:      pb.State_STATE_TIMEOUT,
+			wantState:      proto.State_STATE_TIMEOUT,
 			waitTime:       time.Millisecond * 20,
 			contextTimeout: time.Millisecond * 10,
 			clientErr:      errors.New("Vader Won"),
@@ -327,7 +327,7 @@ func TestContainerManagerWaitFailed(t *testing.T) {
 			if tc.contextTimeout == 0 {
 				ctx = context.Background()
 			}
-			failedChan := make(chan pb.State)
+			failedChan := make(chan proto.State)
 			go mgr.WaitForFailedContainer(ctx, tc.containerID, failedChan)
 			got := <-failedChan