Merge pull request #6 from rschmied/dev

Named configs and version 2.7 support

.github/workflows/go.yml

@@ -11,12 +11,12 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
 
     - name: Set up Go
-      uses: actions/setup-go@v3
+      uses: actions/setup-go@v5
       with:
-        go-version: 1.18
+        go-version: 1.21
 
     - name: Build
       run: go build -v ./...

.gitignore

@@ -1,3 +1,5 @@
 coverage.out
 *.code-workspace
+.envrc
+.tool-versions
 

CHANGELOG.md

@@ -2,6 +2,14 @@
 
 Lists the changes in the gocmlclient package.
 
+## Version 0.1.0
+
+- making a somewhat bigger version bump due to some bigger changes
+- moved logging to log/slog
+- removed all the caching logic / code. It didn't really work well due to races.  In addition, TF doesn't really keep a connection / client over multiple resource calls so the caching was somewhat limited even it would have properly worked (which it did not).
+- named configurations (added with CML 2.7)
+- added some tests for the named configs
+
 ## Version 0.0.23
 
 - added LinkDestroy() method
@@ -34,9 +42,7 @@ fix header and connection error
 ## Version 0.0.17
 
 - added cache control headers to requests
-- return ErrSystemNotReady for Connection refused and 502, also always
-  reset the client's compatibility error property when versionCheck is called
-  so that it always queries the backend.
+- return ErrSystemNotReady for Connection refused and 502, also always reset the client's compatibility error property when versionCheck is called so that it always queries the backend.
 - bumped semver to 3.2.1
 
 ## Version 0.0.16
@@ -49,43 +55,32 @@ fix header and connection error
 
 ## Version 0.0.15
 
-- made node configuration a pointer to differentiate between
-  "no configuration" (null), "empty configuration" and "specific
-  configuration". With a null configuration, the default configuration
-  from the node definition will be inserted if there is one
+- made node configuration a pointer to differentiate between "no configuration" (null), "empty configuration" and "specific configuration". With a null configuration, the default configuration from the node definition will be inserted if there is one
 - added Version var/func, moved NewClient() to New()
 - bump go to 1.19 and vendor deps
 
 ## Version 0.0.12
 
-- Realized that the empty tags removal from 0.0.11 caused a regression.
-  node tags are always returned/set even when there's no tags... in that
-  case, the empty list is returned or needs to be provided. See 0.0.3 comment.
+- Realized that the empty tags removal from 0.0.11 caused a regression. node tags are always returned/set even when there's no tags... in that case, the empty list is returned or needs to be provided. See 0.0.3 comment.
 - Test coverage improvement
 
 ## Version 0.0.8 to 0.0.11
 
 - Added most of the doc string for exported functions.
 - reversed the sorting of images for the image definitions.
-- sort image definitions by their ID. Lists have the newest (highest version)
-  image as the first element.
+- sort image definitions by their ID. Lists have the newest (highest version) image as the first element.
 - updated dependencies.
-- have InterfaceCreate accept a slot value (not a pointer). A negative slot
-  indicates "don't specify a slot", this was previously indicated by nil.
+- have InterfaceCreate accept a slot value (not a pointer). A negative slot indicates "don't specify a slot", this was previously indicated by nil.
 - added more values to the ImageDefinition and Nodedefinition structs.
 - added a link unit test.
 - more node attributes can be updated when a node is DEFINED_ON_CORE
-- NodeCreate removes a node now when the 2nd API call fails. The 2nd call is
-  needed to update certain attributes which are not accepted in the actual
-  create API (POST).
+- NodeCreate removes a node now when the 2nd API call fails. The 2nd call is needed to update certain attributes which are not accepted in the actual create API (POST).
 - move the upper version for the version constraint from <2.6.0 to <3.0.0.
 - omit empty tags on update.
 
 ## Version 0.0.5 to 0.0.7
 
-- refactored the code so that interfaces are read in one go ("data=true"). This
-  without this, only a list of interface IDs is returned by the API. With this,
-  the API returns a list of complete interface object.
+- refactored the code so that interfaces are read in one go ("data=true"). This without this, only a list of interface IDs is returned by the API. With this, the API returns a list of complete interface object.
 - Implement the same approach for nodes (0.0.6).
 - updated dependencies.
 - Due to the data=true option, restrict the code to only work with 2.4.0 and later.
@@ -97,12 +92,9 @@ fix header and connection error
 
 ## Version 0.0.3
 
-- Fixed node tag list update. To delete all tags from a node, an empty tag list
-  must be serialized in the `PATCH` JSON.  This was prevented by having
-  `omitempty` in the struct.  Fixed  
+- Fixed node tag list update. To delete all tags from a node, an empty tag list must be serialized in the `PATCH` JSON.  This was prevented by having `omitempty` in the struct.  Fixed  
 - Also moved the `ctest` cmd fro the terraform provider repo to the code base.
 
 ## Versions prior to 0.0.3
 
-Nothing in particular to be noteworthy -- just huge chunks of initial code
-refactoring.
+Nothing in particular to be noteworthy -- just huge chunks of initial code refactoring.

apiclient.go

@@ -6,7 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"log"
+	"log/slog"
 	"net/http"
 	"net/url"
 	"strings"
@@ -64,7 +64,7 @@ func (c *Client) doAPI(ctx context.Context, req *http.Request, depth int32) ([]b
 	}
 
 	if c.state.get() != stateAuthenticated && c.authRequired(req.URL) {
-		log.Println("needs auth")
+		slog.Info("needs auth")
 		c.state.set(stateAuthenticating)
 		if err := c.jsonGet(ctx, authokAPI, nil, depth); err != nil {
 			return nil, err
@@ -102,7 +102,7 @@ retry:
 	if res.StatusCode == http.StatusUnauthorized {
 		invalid_token := len(c.apiToken) > 0
 		c.apiToken = ""
-		log.Println("need to authenticate")
+		slog.Info("need to authenticate")
 		c.state.set(stateAuthRequired)
 		if !c.userpass.valid() {
 			errmsg := "no credentials provided"

apiclient_test.go

@@ -8,16 +8,14 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-const useCache bool = false
-
 type testClient struct {
 	client *Client
 	mr     *mr.MockResponder
 	ctx    context.Context
 }
 
 func newTestAPIclient() testClient {
-	c := New("https://controller", true, useCache)
+	c := New("https://controller", true)
 	mrClient, ctx := mr.NewMockResponder()
 	c.httpClient = mrClient
 	c.SetUsernamePassword("user", "pass")
@@ -31,7 +29,7 @@ func newAuthedTestAPIclient() testClient {
 }
 
 func TestClient_methoderror(t *testing.T) {
-	c := New("", true, useCache)
+	c := New("", true)
 	err := c.jsonReq(context.Background(), "ü", "###", nil, nil, 0)
 	assert.Error(t, err)
 }

auth.go

@@ -6,7 +6,7 @@ import (
 	"crypto/x509"
 	"encoding/json"
 	"errors"
-	"log"
+	"log/slog"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -36,8 +36,8 @@ func (up userPass) valid() bool {
 	return len(up.Username) > 0 && len(up.Password) > 0
 }
 
-// technically, authokAPI requires auth, but it's used specifically
-// to test whether auth is OK, so it will take a different path
+// technically, authokAPI requires auth, but it's used specifically to test
+// whether auth is OK, so it will take a different path
 func (c *Client) authRequired(api *url.URL) bool {
 	url := api.String()
 	return !(strings.HasSuffix(url, authAPI) ||
@@ -56,16 +56,16 @@ func (c *Client) authenticate(ctx context.Context, userpass userPass, depth int3
 	if err != nil {
 		return err
 	}
-	log.Printf("user id %s, is admin: %s", auth.ID, strconv.FormatBool(auth.Admin))
+	slog.Info("user auth", "id", auth.ID, "is_admin", strconv.FormatBool(auth.Admin))
 	c.apiToken = auth.Token
 	return nil
 }
 
 // SetToken sets a specific API token to be used. A token takes precedence over
-// a username/password. However, if the token expires, the username/password are
-// used to authorize the client again. An error is raised if no token and no
-// username/password are provided or if the token expires when no username/password
-// are set.
+// a username/password. However, if the token expires, the username/password
+// are used to authorize the client again. An error is raised if no token and
+// no username/password are provided or if the token expires when no
+// username/password are set.
 func (c *Client) SetToken(token string) {
 	c.apiToken = token
 }

auth_test.go

@@ -151,20 +151,20 @@ func TestClient_token_auth(t *testing.T) {
 }
 
 func TestClient_SetToken(t *testing.T) {
-	c := New("https://bla.bla", true, useCache)
+	c := New("https://bla.bla", true)
 	c.SetToken("qwe")
 	assert.Equal(t, "qwe", c.apiToken)
 }
 
 func TestClient_SetUsernamePassword(t *testing.T) {
-	c := New("https://bla.bla", true, useCache)
+	c := New("https://bla.bla", true)
 	c.SetUsernamePassword("user", "pass")
 	assert.Equal(t, "user", c.userpass.Username)
 	assert.Equal(t, "pass", c.userpass.Password)
 }
 
 func TestClient_SetCACert(t *testing.T) {
-	c := New("https://bla.bla", true, useCache)
+	c := New("https://bla.bla", true)
 	err := c.SetCACert([]byte("crapdata"))
 	assert.EqualError(t, err, "failed to parse root certificate")
 	testCA := "testdata/ca.pem"

cmd/ctest/main.go

@@ -2,23 +2,36 @@ package main
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
-	"log"
+	"fmt"
+	"log/slog"
 	"os"
+	"time"
 
+	"github.com/lmittmann/tint"
 	cmlclient "github.com/rschmied/gocmlclient"
 )
 
 func main() {
+	// set global logger with custom options
+	slog.SetDefault(slog.New(
+		tint.NewHandler(os.Stderr, &tint.Options{
+			AddSource:  true,
+			Level:      slog.LevelDebug,
+			TimeFormat: time.RFC822,
+		}),
+	))
+
 	// address and lab id
 	host, found := os.LookupEnv("CML_HOST")
 	if !found {
-		log.Println("CML_HOST env var not found!")
+		slog.Error("CML_HOST env var not found!")
 		return
 	}
 	// labID, found := os.LookupEnv("CML_LABID")
 	// if !found {
-	// 	log.Println("CML_LABID env var not found!")
+	// 	slog.Error("CML_LABID env var not found!")
 	// 	return
 	// }
 	// _ = labID
@@ -28,11 +41,11 @@ func main() {
 	password, pass_found := os.LookupEnv("CML_PASSWORD")
 	token, token_found := os.LookupEnv("CML_TOKEN")
 	if !(token_found || (user_found && pass_found)) {
-		log.Println("either CML_TOKEN or CML_USERNAME and CML_PASSWORD env vars must be present!")
+		slog.Error("either CML_TOKEN or CML_USERNAME and CML_PASSWORD env vars must be present!")
 		return
 	}
 	ctx := context.Background()
-	client := cmlclient.New(host, true, false)
+	client := cmlclient.New(host, false)
 	// if err := client.Ready(ctx); err != nil {
 	// 	log.Fatal(err)
 	// }
@@ -86,16 +99,49 @@ func main() {
 	// result, err := client.UserGroups(ctx, "cc42bd56-1dc6-445c-b7e7-569b0a8b0c94")
 	err := client.Ready(ctx)
 	if errors.Is(err, cmlclient.ErrSystemNotReady) {
-		log.Println("it is not ready")
+		slog.Error("it is not ready")
+		return
 	}
 	if err != nil && !errors.Is(err, cmlclient.ErrSystemNotReady) {
-		log.Println(err)
+		slog.Error("ready", slog.Any("error", err))
+		return
+	}
+	/* node := &cmlclient.Node{
+		// ID:    "28ec08ec-483a-415a-a3ed-625b0d45bef0",
+		// ID:    "8116a609-8b68-4e0f-a196-5225da9f05c0",
+		ID:    "0577f1c4-4907-4c49-a4fd-c6daa61b6e78",
+		LabID: "2b7435f2-b247-4cc8-8509-6b0d0f593c4c",
+	}
+	node, err = client.NodeGet(ctx, node, false)
+	if err != nil {
+		slog.Error("nodeget", slog.Any("error", err))
 		return
 	}
 
-	// je, err := json.Marshal(result)
-	// if err != nil {
-	// 	log.Println(err)
-	// }
-	// fmt.Println(string(je))
+	je, err := json.Marshal(node)
+	if err != nil {
+		slog.Error("marshal", slog.Any("error", err))
+		return
+	}
+	fmt.Println(string(je)) */
+
+	lab, err := client.LabGet(ctx, "2b7435f2-b247-4cc8-8509-6b0d0f593c4c", true)
+	if err != nil {
+		slog.Error("get", slog.Any("error", err))
+		return
+	}
+
+	for _, v := range lab.Nodes {
+		if v.Configuration != nil {
+			fmt.Printf("[1] %T: %s\n", v.Configuration, *v.Configuration)
+		}
+		fmt.Printf("[2] %T: %+v\n", v.Configurations, v.Configurations)
+	}
+	return
+	je, err := json.Marshal(lab)
+	if err != nil {
+		slog.Error("marshal", slog.Any("error", err))
+		return
+	}
+	fmt.Println(string(je))
 }

cml.go

@@ -19,14 +19,13 @@ type Client struct {
 	compatibilityErr error
 	state            *apiClientState
 	mu               sync.RWMutex
-	labCache         map[string]*Lab
-	useCache         bool
+	useNamedConfigs  bool
 	version          string
 }
 
-// New returns a new CML client instance. The host must be a valid URL including
-// scheme (https://).
-func New(host string, insecure, useCache bool) *Client {
+// New returns a new CML client instance. The host must be a valid URL
+// including scheme (https://).
+func New(host string, insecure bool) *Client {
 	tr := http.DefaultTransport.(*http.Transport)
 	tr.TLSClientConfig = &tls.Config{
 		InsecureSkipVerify: insecure,
@@ -44,7 +43,6 @@ func New(host string, insecure, useCache bool) *Client {
 		},
 		compatibilityErr: nil,
 		state:            newState(),
-		labCache:         make(map[string]*Lab),
-		useCache:         useCache,
+		useNamedConfigs:  false,
 	}
 }

error.go

@@ -3,6 +3,7 @@ package cmlclient
 import "errors"
 
 var (
-	ErrSystemNotReady  = errors.New("system not ready")
-	ErrElementNotFound = errors.New("element not found")
+	ErrSystemNotReady       = errors.New("system not ready")
+	ErrElementNotFound      = errors.New("element not found")
+	ErrNoNamedConfigSupport = errors.New("backend does not support named configs")
 )

go.mod

@@ -1,9 +1,10 @@
 module github.com/rschmied/gocmlclient
 
-go 1.20
+go 1.21
 
 require (
 	github.com/Masterminds/semver/v3 v3.2.1
+	github.com/lmittmann/tint v1.0.4
 	github.com/rschmied/mockresponder v1.0.4
 	github.com/stretchr/testify v1.8.2
 	golang.org/x/sync v0.7.0