Stop serving and fetching the tls cert used for gRPC (#584)

## Description

- Stops serving the gRPC cert via http and stops fetching it.
- Cleans up a ton of code that was confusing to read in the clients due to dealing with the gRPC certificate.
- Removes a few Config structs that are passed into funcs
  These were used to get around having lots of parameters and the long lines that caused, but now are just one or two values.

## Why is this needed

The whole serving of the gRPC certificate via http as `/cert` should never have made it to tinkerbell.
It's too easy to use incorrectly and fall into a sense of security that may not be there.
Its also a pain to actually use in production when following modern best practices of short lived TLS certificates.
Clients can't use gRPC's/Go's builtin certificate handling of when certs are rotated.

This is not a lot of good, yet a bunch of bad only to make a corner case easier at the expense of normal route.
`/cert` makes self-sigend certs "easy"ish (see cert rotation issue still) but it's just not worth it.
If someone can handle the implications of self-signed certs in production (#567 can be used for dev envs) then they can figure out how to embed their CA into hook or roll their own tink-worker environment.

## How Has This Been Tested?

Compiles and tests pass.

## How are existing users impacted? What migration steps/scripts do we need?

This is a breaking change for out-of-tree clients, I think its worth it compared to the ops / security benefits.

Fixes #324

.envrc

@@ -4,4 +4,3 @@ PATH_add bin
 path_add GOBIN bin
 
 export TINKERBELL_GRPC_AUTHORITY=127.0.0.1:42113
-export TINKERBELL_CERT_URL=http://127.0.0.1:42114/cert

client/main.go

@@ -1,14 +1,8 @@
 package client
 
 import (
-	"crypto/x509"
-	"io/ioutil"
-	"log"
-	"net/http"
-
 	"github.com/packethost/pkg/env"
 	"github.com/pkg/errors"
-	"github.com/spf13/pflag"
 	"github.com/tinkerbell/tink/protos/hardware"
 	"github.com/tinkerbell/tink/protos/template"
 	"github.com/tinkerbell/tink/protos/workflow"
@@ -41,55 +35,16 @@ func NewFullClient(conn grpc.ClientConnInterface) *FullClient {
 	}
 }
 
-type ConnOptions struct {
-	CertURL       string
-	GRPCAuthority string
-	TLS           bool
-}
-
-func (o *ConnOptions) SetFlags(flagSet *pflag.FlagSet) {
-	flagSet.StringVar(&o.CertURL, "tinkerbell-cert-url", "http://127.0.0.1:42114/cert", "The URL where the certificate is located")
-	flagSet.StringVar(&o.GRPCAuthority, "tinkerbell-grpc-authority", "127.0.0.1:42113", "Connection info for tink-server")
-	flagSet.BoolVar(&o.TLS, "tinkerbell-tls", true, "Connect to server via TLS or not")
-}
-
-// This function is bad and ideally should be removed, but for now it moves all the bad into one place.
-// This is the legacy of packethost/cacher running behind an ingress that couldn't terminate TLS on behalf
-// of GRPC. All of this functionality should be ripped out in favor of either using trusted certificates
-// or moving the establishment of trust in the certificate out to the environment (or running in no-tls mode
-// e.g. for development.)
-func grpcCredentialFromCertEndpoint(url string) (credentials.TransportCredentials, error) {
-	resp, err := http.Get(url)
-	if err != nil {
-		return nil, errors.Wrap(err, "fetch cert")
+func NewClientConn(authority string, tls bool) (*grpc.ClientConn, error) {
+	var creds grpc.DialOption
+	if tls {
+		creds = grpc.WithTransportCredentials(credentials.NewTLS(nil))
+	} else {
+		creds = grpc.WithInsecure()
 	}
-	defer resp.Body.Close()
 
-	certs, err := ioutil.ReadAll(resp.Body)
-	if err != nil {
-		return nil, errors.Wrap(err, "read cert")
-	}
-
-	cp := x509.NewCertPool()
-	ok := cp.AppendCertsFromPEM(certs)
-	if !ok {
-		return nil, errors.Wrap(err, "parse cert")
-	}
-
-	return credentials.NewClientTLSFromCert(cp, ""), nil
-}
-
-func NewClientConn(opt *ConnOptions) (*grpc.ClientConn, error) {
-	method := grpc.WithInsecure()
-	if opt.TLS {
-		creds, err := grpcCredentialFromCertEndpoint(opt.CertURL)
-		if err != nil {
-			return nil, err
-		}
-		method = grpc.WithTransportCredentials(creds)
-	}
-	conn, err := grpc.Dial(opt.GRPCAuthority,
-		method,
+	conn, err := grpc.Dial(authority,
+		creds,
 		grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()),
 		grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()),
 	)
@@ -101,22 +56,13 @@ func NewClientConn(opt *ConnOptions) (*grpc.ClientConn, error) {
 
 // GetConnection returns a gRPC client connection.
 func GetConnection() (*grpc.ClientConn, error) {
-	opts := ConnOptions{
-		CertURL:       env.Get("TINKERBELL_CERT_URL"),
-		GRPCAuthority: env.Get("TINKERBELL_GRPC_AUTHORITY"),
-		TLS:           env.Bool("TINKERBELL_TLS", true),
-	}
-
-	if opts.GRPCAuthority == "" {
+	authority := env.Get("TINKERBELL_GRPC_AUTHORITY")
+	if authority == "" {
 		return nil, errors.New("undefined TINKERBELL_GRPC_AUTHORITY")
 	}
 
-	if opts.TLS {
-		if opts.CertURL == "" {
-			return nil, errors.New("undefined TINKERBELL_CERT_URL")
-		}
-	}
-	return NewClientConn(&opts)
+	tls := env.Bool("TINKERBELL_TLS", true)
+	return NewClientConn(authority, tls)
 }
 
 // Setup : create a connection to server.
@@ -135,16 +81,38 @@ func Setup() error {
 func TinkHardwareClient() (hardware.HardwareServiceClient, error) {
 	conn, err := GetConnection()
 	if err != nil {
-		log.Fatal(err)
+		return nil, err
 	}
 	return hardware.NewHardwareServiceClient(conn), nil
 }
 
+// TinkTemplateClient creates a new hardware client.
+func TinkTemplateClient() (template.TemplateServiceClient, error) {
+	conn, err := GetConnection()
+	if err != nil {
+		return nil, err
+	}
+	return template.NewTemplateServiceClient(conn), nil
+}
+
 // TinkWorkflowClient creates a new workflow client.
 func TinkWorkflowClient() (workflow.WorkflowServiceClient, error) {
 	conn, err := GetConnection()
 	if err != nil {
-		log.Fatal(err)
+		return nil, err
 	}
 	return workflow.NewWorkflowServiceClient(conn), nil
 }
+
+// TinkFullClient creates a new full client.
+func TinkFullClient() (FullClient, error) {
+	conn, err := GetConnection()
+	if err != nil {
+		return FullClient{}, err
+	}
+	return FullClient{
+		HardwareClient: hardware.NewHardwareServiceClient(conn),
+		TemplateClient: template.NewTemplateServiceClient(conn),
+		WorkflowClient: workflow.NewWorkflowServiceClient(conn),
+	}, nil
+}

cmd/tink-cli/cmd/delete/delete.go

@@ -7,25 +7,14 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/tinkerbell/tink/client"
-	"google.golang.org/grpc"
+	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/internal/clientctx"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
 type Options struct {
 	// DeleteByID is used to delete a resource
 	DeleteByID func(context.Context, *client.FullClient, string) (interface{}, error)
-
-	clientConnOpt *client.ConnOptions
-	fullClient    *client.FullClient
-}
-
-func (o *Options) SetClientConnOpt(co *client.ConnOptions) {
-	o.clientConnOpt = co
-}
-
-func (o *Options) SetFullClient(cl *client.FullClient) {
-	o.fullClient = cl
 }
 
 const shortDescr = "delete one or more resources"
@@ -73,35 +62,14 @@ func NewDeleteCommand(opt Options) *cobra.Command {
 		Long:                  longDescr,
 		Example:               exampleDescr,
 		DisableFlagsInUseLine: true,
-		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-			if opt.fullClient != nil {
-				return nil
-			}
-			return nil
-		},
-		PreRunE: func(cmd *cobra.Command, args []string) error {
-			if opt.fullClient == nil {
-				var err error
-				var conn *grpc.ClientConn
-				conn, err = client.NewClientConn(opt.clientConnOpt)
-				if err != nil {
-					fmt.Fprintf(cmd.ErrOrStderr(), "Flag based client configuration failed with err: %s. Trying with env var legacy method...", err)
-					// Fallback to legacy Setup via env var
-					conn, err = client.GetConnection()
-					if err != nil {
-						return errors.Wrap(err, "failed to setup connection to tink-server")
-					}
-				}
-				opt.SetFullClient(client.NewFullClient(conn))
-			}
-			return nil
-		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if opt.DeleteByID == nil {
 				return errors.New("option DeleteByID is not implemented for this resource yet. Please have a look at the issue in GitHub or open a new one")
 			}
+
+			client := clientctx.Get(cmd.Context())
 			for _, requestedID := range args {
-				_, err := opt.DeleteByID(cmd.Context(), opt.fullClient, requestedID)
+				_, err := opt.DeleteByID(cmd.Context(), client, requestedID)
 				if err != nil {
 					if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
 						fmt.Fprintf(cmd.ErrOrStderr(), "Error\t%s\tnot found\n", requestedID)
@@ -114,9 +82,5 @@ func NewDeleteCommand(opt Options) *cobra.Command {
 			return nil
 		},
 	}
-	if opt.clientConnOpt == nil {
-		opt.SetClientConnOpt(&client.ConnOptions{})
-	}
-	opt.clientConnOpt.SetFlags(cmd.PersistentFlags())
 	return cmd
 }

cmd/tink-cli/cmd/delete/delete_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/tinkerbell/tink/client"
+	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/internal/clientctx"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -77,12 +78,11 @@ func TestNewDeleteCommand(t *testing.T) {
 	for _, test := range table {
 		t.Run(test.name, func(t *testing.T) {
 			stdout := bytes.NewBufferString("")
-			test.opt.SetFullClient(&client.FullClient{})
 			cmd := NewDeleteCommand(test.opt)
 			cmd.SetOut(stdout)
 			cmd.SetErr(stdout)
 			cmd.SetArgs(test.args)
-			err := cmd.Execute()
+			err := cmd.ExecuteContext(clientctx.Set(context.Background(), &client.FullClient{}))
 			if err != nil {
 				t.Error(err)
 			}

cmd/tink-cli/cmd/get/get.go

@@ -9,7 +9,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/tinkerbell/tink/client"
-	"google.golang.org/grpc"
+	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/internal/clientctx"
 )
 
 type Options struct {
@@ -22,24 +22,13 @@ type Options struct {
 	// PopulateTable populates a table with the data retrieved with the RetrieveData function.
 	PopulateTable func([]interface{}, table.Writer) error
 
-	clientConnOpt *client.ConnOptions
-	fullClient    *client.FullClient
-
 	// Format specifies the format you want the list of resources printed
 	// out. By default it is table but it can be JSON ar CSV.
 	Format string
 	// NoHeaders does not print the header line
 	NoHeaders bool
 }
 
-func (o *Options) SetClientConnOpt(co *client.ConnOptions) {
-	o.clientConnOpt = co
-}
-
-func (o *Options) SetFullClient(cl *client.FullClient) {
-	o.fullClient = cl
-}
-
 const shortDescr = `display one or many resources`
 
 const longDescr = `Prints a table containing the most important information about a specific
@@ -64,49 +53,27 @@ func NewGetCommand(opt Options) *cobra.Command {
 		Long:                  longDescr,
 		Example:               exampleDescr,
 		DisableFlagsInUseLine: true,
-		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-			if opt.fullClient != nil {
-				return nil
-			}
-			return nil
-		},
-		PreRunE: func(cmd *cobra.Command, args []string) error {
-			if opt.fullClient == nil {
-				var err error
-				var conn *grpc.ClientConn
-				conn, err = client.NewClientConn(opt.clientConnOpt)
-				if err != nil {
-					fmt.Fprintf(cmd.ErrOrStderr(), "Flag based client configuration failed with err: %s. Trying with env var legacy method...", err)
-					// Fallback to legacy Setup via env var
-					conn, err = client.GetConnection()
-					if err != nil {
-						return errors.Wrap(err, "failed to setup connection to tink-server")
-					}
-				}
-				opt.SetFullClient(client.NewFullClient(conn))
-			}
-			return nil
-		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			var err error
 			var data []interface{}
 
 			t := table.NewWriter()
 			t.SetOutputMirror(cmd.OutOrStdout())
 
+			client := clientctx.Get(cmd.Context())
 			if len(args) != 0 {
 				if opt.RetrieveByID == nil {
 					return errors.New("option RetrieveByID is not implemented for this resource yet. Please have a look at the issue in GitHub or open a new one")
 				}
 				for _, requestedID := range args {
-					s, err := opt.RetrieveByID(cmd.Context(), opt.fullClient, requestedID)
+					s, err := opt.RetrieveByID(cmd.Context(), client, requestedID)
 					if err != nil {
 						continue
 					}
 					data = append(data, s)
 				}
 			} else {
-				data, err = opt.RetrieveData(cmd.Context(), opt.fullClient)
+				data, err = opt.RetrieveData(cmd.Context(), client)
 			}
 			if err != nil {
 				return err
@@ -148,9 +115,5 @@ func NewGetCommand(opt Options) *cobra.Command {
 	}
 	cmd.PersistentFlags().StringVarP(&opt.Format, "format", "", "table", "The format you expect the list to be printed out. Currently supported format are table, JSON and CSV")
 	cmd.PersistentFlags().BoolVar(&opt.NoHeaders, "no-headers", false, "Table contains an header with the columns' name. You can disable it from being printed out")
-	if opt.clientConnOpt == nil {
-		opt.SetClientConnOpt(&client.ConnOptions{})
-	}
-	opt.clientConnOpt.SetFlags(cmd.PersistentFlags())
 	return cmd
 }

cmd/tink-cli/cmd/get/get_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/jedib0t/go-pretty/table"
 	"github.com/spf13/cobra"
 	"github.com/tinkerbell/tink/client"
+	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/internal/clientctx"
 )
 
 func TestNewGetCommand(t *testing.T) {
@@ -181,11 +182,10 @@ func TestNewGetCommand(t *testing.T) {
 				t.Skip(s.Skip)
 			}
 			stdout := bytes.NewBufferString("")
-			s.Opt.SetFullClient(&client.FullClient{})
 			cmd := NewGetCommand(s.Opt)
 			cmd.SetOut(stdout)
 			cmd.SetArgs(s.Args)
-			err := cmd.Execute()
+			err := cmd.ExecuteContext(clientctx.Set(context.Background(), &client.FullClient{}))
 			if err != nil {
 				t.Error(err)
 			}

cmd/tink-cli/cmd/hardware/get_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/tinkerbell/tink/client"
 	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/get"
+	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/internal/clientctx"
 	hardware_proto "github.com/tinkerbell/tink/protos/hardware"
 	"google.golang.org/grpc"
 )
@@ -93,11 +94,10 @@ func TestGetHardware(t *testing.T) {
 			}
 			stdout := bytes.NewBufferString("")
 			g := NewGetOptions()
-			g.SetFullClient(cl)
 			cmd := get.NewGetCommand(g)
 			cmd.SetOut(stdout)
 			cmd.SetArgs(s.Args)
-			err := cmd.Execute()
+			err := cmd.ExecuteContext(clientctx.Set(context.Background(), cl))
 			if err != nil {
 				t.Error(err)
 			}