Adressed pr comments

api/orb/client.go

@@ -2,18 +2,24 @@ package orb
 
 import (
 	"fmt"
+	"io"
+	"os"
 	"sync"
 
 	"github.com/CircleCI-Public/circleci-cli/api"
 	"github.com/CircleCI-Public/circleci-cli/api/graphql"
 	"github.com/CircleCI-Public/circleci-cli/settings"
+	"github.com/pkg/errors"
 )
 
 var (
-	once   sync.Once
-	client Client
+	once        sync.Once
+	client      Client
+	clientError error
 )
 
+type clientVersion string
+
 // ConfigResponse is a structure that matches the result of the GQL
 // query, so that we can use mapstructure to convert from
 // nested maps to a strongly typed struct.
@@ -27,27 +33,43 @@ type Client interface {
 	OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error)
 }
 
-func GetClient(config *settings.Config) Client {
+func GetClient(config *settings.Config) (Client, error) {
 	once.Do(func() {
-		createClient(config)
+		client, clientError = newClient(config)
 	})
-	return client
+
+	return client, clientError
 }
 
-func createClient(config *settings.Config) {
+func newClient(config *settings.Config) (Client, error) {
 	gql := graphql.NewClient(config.HTTPClient, config.Host, config.Endpoint, config.Token, config.Debug)
 
-	ok, err := orbQueryHandleOwnerId(gql)
+	clientVersion, err := detectClientVersion(gql)
 	if err != nil {
-		fmt.Printf("While requesting orb server: %s", err)
-		return
-	} else if ok {
-		client = &latestClient{gql}
-	} else {
-		client = &deprecatedClient{gql}
+		return nil, err
+	}
+
+	switch clientVersion {
+	case v1_string:
+		return &v1Client{gql}, nil
+	case v2_string:
+		return &v2Client{gql}, nil
+	default:
+		return nil, fmt.Errorf("Unable to recognise your server orb API")
 	}
 }
 
+func detectClientVersion(gql *graphql.Client) (clientVersion, error) {
+	handlesOwnerId, err := orbQueryHandleOwnerId(gql)
+	if err != nil {
+		return "", err
+	}
+	if !handlesOwnerId {
+		return v1_string, nil
+	}
+	return v2_string, nil
+}
+
 type OrbIntrospectionResponse struct {
 	Schema struct {
 		Query struct {
@@ -62,22 +84,21 @@ type OrbIntrospectionResponse struct {
 }
 
 func orbQueryHandleOwnerId(gql *graphql.Client) (bool, error) {
-	query := `
-query ValidateOrb {
-  __schema {
-    queryType {
-      fields(includeDeprecated: true) {
-        name
-        args {
-          name
-          __typename
-          type {
-            name
-          }
-        }
-      }
-    }
-  }
+	query := `query IntrospectionQuery {
+	_schema {
+		queryType {
+			fields(includeDeprecated: true) {
+				name
+				args {
+					name
+					__typename
+					type {
+						name
+					}
+				}
+			}
+		}
+	}
 }`
 	request := graphql.NewRequest(query)
 	response := OrbIntrospectionResponse{}
@@ -103,3 +124,19 @@ query ValidateOrb {
 
 	return false, nil
 }
+
+func loadYaml(path string) (string, error) {
+	var err error
+	var config []byte
+	if path == "-" {
+		config, err = io.ReadAll(os.Stdin)
+	} else {
+		config, err = os.ReadFile(path)
+	}
+
+	if err != nil {
+		return "", errors.Wrapf(err, "Could not load config file at %s", path)
+	}
+
+	return string(config), nil
+}

api/orb/deprecated.go → api/orb/v1_client.go

@@ -6,11 +6,15 @@ import (
 	"github.com/pkg/errors"
 )
 
-type deprecatedClient struct {
+// This client makes request to servers that **DON'T** have the field `ownerId` in the GraphQL query method: `orbConfig`
+
+const v1_string clientVersion = "v1"
+
+type v1Client struct {
 	gql *graphql.Client
 }
 
-func (deprecated *deprecatedClient) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) {
+func (client *v1Client) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) {
 	if ownerId != "" {
 		return nil, errors.New("Your version of Server does not support validating orbs that refer to other private orbs. Please see the README for more information on server compatibility: https://github.com/CircleCI-Public/circleci-cli#server-compatibility")
 	}
@@ -22,24 +26,23 @@ func (deprecated *deprecatedClient) OrbQuery(configPath string, ownerId string)
 		return nil, err
 	}
 
-	query := `
-		query ValidateOrb ($config: String!) {
-			orbConfig(orbYaml: $config) {
-				valid,
-				errors { message },
-				sourceYaml,
-				outputYaml
-			}
-		}`
+	query := `query ValidateOrb ($config: String!) {
+	orbConfig(orbYaml: $config) {
+		valid,
+		errors { message },
+		sourceYaml,
+		outputYaml
+	}
+}`
 
 	request := graphql.NewRequest(query)
 	request.Var("config", configContent)
 
-	request.SetToken(deprecated.gql.Token)
+	request.SetToken(client.gql.Token)
 
-	err = deprecated.gql.Run(request, &response)
+	err = client.gql.Run(request, &response)
 	if err != nil {
-		return nil, errors.Wrap(err, "Unable to validate config")
+		return nil, errors.Wrap(err, "Validating config")
 	}
 
 	if len(response.OrbConfig.ConfigResponse.Errors) > 0 {

api/orb/latest.go → api/orb/v2_client.go

@@ -6,38 +6,42 @@ import (
 	"github.com/pkg/errors"
 )
 
-type latestClient struct {
+// This client makes request to servers that **DO** have the field `ownerId` in the GraphQL query method: `orbConfig`
+
+const v2_string clientVersion = "v2"
+
+type v2Client struct {
 	gql *graphql.Client
 }
 
-func (latest *latestClient) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) {
+func (client *v2Client) OrbQuery(configPath string, ownerId string) (*api.ConfigResponse, error) {
 	var response QueryResponse
 
 	configContent, err := loadYaml(configPath)
 	if err != nil {
 		return nil, err
 	}
 
-	query := `
-		query ValidateOrb ($config: String!, $owner: UUID) {
-			orbConfig(orbYaml: $config, ownerId: $owner) {
-				valid,
-				errors { message },
-				sourceYaml,
-				outputYaml
-			}
-		}`
+	query := `query ValidateOrb ($config: String!, $owner: UUID) {
+	orbConfig(orbYaml: $config, ownerId: $owner) {
+		valid,
+		errors { message },
+		sourceYaml,
+		outputYaml
+	}
+}`
+
 	request := graphql.NewRequest(query)
 	request.Var("config", configContent)
 
 	if ownerId != "" {
 		request.Var("owner", ownerId)
 	}
-	request.SetToken(latest.gql.Token)
+	request.SetToken(client.gql.Token)
 
-	err = latest.gql.Run(request, &response)
+	err = client.gql.Run(request, &response)
 	if err != nil {
-		return nil, errors.Wrap(err, "Unable to validate config")
+		return nil, errors.Wrap(err, "Validating config")
 	}
 
 	if len(response.OrbConfig.ConfigResponse.Errors) > 0 {

cmd/orb.go

@@ -733,9 +733,9 @@ func validateOrb(opts orbOptions, org orbOrgOptions) error {
 		return fmt.Errorf("failed to get the appropriate org-id: %s", err.Error())
 	}
 
-	client := orb.GetClient(opts.cfg)
-	if client == nil {
-		return fmt.Errorf("Unable to validate orb")
+	client, err := orb.GetClient(opts.cfg)
+	if err != nil {
+		return errors.Wrap(err, "Getting orb client")
 	}
 	_, err = client.OrbQuery(opts.args[0], orgId)
 
@@ -759,9 +759,9 @@ func processOrb(opts orbOptions, org orbOrgOptions) error {
 		return fmt.Errorf("failed to get the appropriate org-id: %s", err.Error())
 	}
 
-	client := orb.GetClient(opts.cfg)
-	if client == nil {
-		return fmt.Errorf("Unable to validate orb")
+	client, err := orb.GetClient(opts.cfg)
+	if err != nil {
+		return errors.Wrap(err, "Getting orb client")
 	}
 	response, err := client.OrbQuery(opts.args[0], orgId)