Minor cleanup of client/main.go (#570)

## Description

shell.nix: include docker-compose for Makefile compatibility
client: purge unused NewFullClientFromGlobal function
client: factor out grpcCredentialFromCertEndpoint from two locations into function

## Why is this needed

This removes unused code, and factors out duplicate code into a single function. The codebase is better than it was before.

## How Has This Been Tested?

```console
make images
make run-stack
cat focal.yaml | docker exec -i tink_tink-cli_1 tink template create
docker exec -i tink_tink-cli_1 tink template get
docker exec -i tink_tink-cli_1 tink template delete <uuid>
```

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

N/A

## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade

client/main.go

@@ -31,29 +31,8 @@ type FullClient struct {
 	HardwareClient hardware.HardwareServiceClient
 }
 
-// NewFullClientFromGlobal is a dirty hack that returns a FullClient using the
-// global variables exposed by the client package. Globals should be avoided
-// and we will deprecate them at some point replacing this function with
-// NewFullClient. If you are starting a new project please use NewFullClient instead.
-func NewFullClientFromGlobal() (*FullClient, error) {
-	// This is required because we use init() too often, even more in the
-	// CLI and based on where you are sometime the clients are not initialised
-	if TemplateClient == nil {
-		err := Setup()
-		if err != nil {
-			panic(err)
-		}
-	}
-	return &FullClient{
-		TemplateClient: TemplateClient,
-		WorkflowClient: WorkflowClient,
-		HardwareClient: HardwareClient,
-	}, nil
-}
-
 // NewFullClient returns a FullClient. A structure that contains all the
-// clients made available from tink-server. This is the function you should use
-// instead of NewFullClientFromGlobal that will be deprecated soon.
+// clients made available from tink-server.
 func NewFullClient(conn grpc.ClientConnInterface) *FullClient {
 	return &FullClient{
 		TemplateClient: template.NewTemplateServiceClient(conn),
@@ -72,25 +51,34 @@ func (o *ConnOptions) SetFlags(flagSet *pflag.FlagSet) {
 	flagSet.StringVar(&o.GRPCAuthority, "tinkerbell-grpc-authority", "127.0.0.1:42113", "Link to tink-server grcp api")
 }
 
-func NewClientConn(opt *ConnOptions) (*grpc.ClientConn, error) {
-	resp, err := http.Get(opt.CertURL)
+// 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 insecure 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")
 	}
 	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
+}
 
-	creds := credentials.NewClientTLSFromCert(cp, "")
+func NewClientConn(opt *ConnOptions) (*grpc.ClientConn, error) {
+	creds, err := grpcCredentialFromCertEndpoint(opt.CertURL)
+	if err != nil {
+		return nil, errors.Wrap(err, "obtain trusted certificate")
+	}
 	conn, err := grpc.Dial(opt.GRPCAuthority, grpc.WithTransportCredentials(creds))
 	if err != nil {
 		return nil, errors.Wrap(err, "connect to tinkerbell server")
@@ -104,28 +92,16 @@ func GetConnection() (*grpc.ClientConn, error) {
 	if certURL == "" {
 		return nil, errors.New("undefined TINKERBELL_CERT_URL")
 	}
-	resp, err := http.Get(certURL)
-	if err != nil {
-		return nil, errors.Wrap(err, "fetch cert")
-	}
-	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")
-	}
 
 	grpcAuthority := os.Getenv("TINKERBELL_GRPC_AUTHORITY")
 	if grpcAuthority == "" {
 		return nil, errors.New("undefined TINKERBELL_GRPC_AUTHORITY")
 	}
-	creds := credentials.NewClientTLSFromCert(cp, "")
+
+	creds, err := grpcCredentialFromCertEndpoint(certURL)
+	if err != nil {
+		return nil, errors.Wrap(err, "obtain trusted certificate")
+	}
 	conn, err := grpc.Dial(grpcAuthority,
 		grpc.WithTransportCredentials(creds),
 		grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()),

shell.nix

@@ -11,6 +11,7 @@ with pkgs;
 
 mkShell {
   buildInputs = [
+    docker-compose
     git
     gnumake
     gnused
@@ -20,8 +21,8 @@ mkShell {
     nodePackages.prettier
     protobuf
     python3Packages.codespell
-    shfmt
     shellcheck
+    shfmt
     vagrant
   ];
 }