add tims review suggestions

Author: AdamVB

providers/jamf/connection/connection.go

@@ -19,13 +19,19 @@ type JamfConnection struct {
 	Conf   *inventory.Config
 	asset  *inventory.Asset
 	Client *jamfpro.Client
+
+	// localUserAccounts caches per-computer local user accounts from the
+	// initial inventory fetch, keyed by computer ID. This avoids N+1 API
+	// calls when iterating computerInventory then accessing localUserAccounts.
+	localUserAccounts map[string][]jamfpro.ComputerInventorySubsetLocalUserAccount
 }
 
 func NewJamfConnection(id uint32, asset *inventory.Asset, conf *inventory.Config) (*JamfConnection, error) {
 	conn := &JamfConnection{
-		Connection: plugin.NewConnection(id, asset),
-		Conf:       conf,
-		asset:      asset,
+		Connection:        plugin.NewConnection(id, asset),
+		Conf:              conf,
+		asset:             asset,
+		localUserAccounts: make(map[string][]jamfpro.ComputerInventorySubsetLocalUserAccount),
 	}
 
 	// Extract credentials and options from conf
@@ -88,3 +94,16 @@ func (j *JamfConnection) Identifier() string {
 	domain := j.Conf.Options["instance_domain"]
 	return "//platformid.api.mondoo.app/runtime/jamf/" + strings.ToLower(domain)
 }
+
+// CacheLocalUserAccounts stores local user accounts for a computer ID,
+// populated during the initial inventory fetch.
+func (j *JamfConnection) CacheLocalUserAccounts(computerID string, accounts []jamfpro.ComputerInventorySubsetLocalUserAccount) {
+	j.localUserAccounts[computerID] = accounts
+}
+
+// GetCachedLocalUserAccounts retrieves cached local user accounts for a
+// computer ID. Returns nil, false if no cache entry exists.
+func (j *JamfConnection) GetCachedLocalUserAccounts(computerID string) ([]jamfpro.ComputerInventorySubsetLocalUserAccount, bool) {
+	accounts, ok := j.localUserAccounts[computerID]
+	return accounts, ok
+}

providers/jamf/connection/connection_test.go

@@ -0,0 +1,103 @@
+// Copyright Mondoo, Inc. 2024, 2026
+// SPDX-License-Identifier: BUSL-1.1
+
+package connection
+
+import (
+	"testing"
+
+	"github.com/deploymenttheory/go-api-sdk-jamfpro/sdk/jamfpro"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.mondoo.com/mql/v13/providers-sdk/v1/inventory"
+	"go.mondoo.com/mql/v13/providers-sdk/v1/vault"
+)
+
+func TestNewJamfConnection_MissingCredentials(t *testing.T) {
+	_, err := NewJamfConnection(0, &inventory.Asset{}, &inventory.Config{
+		Options: map[string]string{},
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "missing required Jamf credentials")
+}
+
+func TestNewJamfConnection_MissingDomain(t *testing.T) {
+	cred := &vault.Credential{
+		Type:   vault.CredentialType_password,
+		User:   "client-id",
+		Secret: []byte("client-secret"),
+	}
+	_, err := NewJamfConnection(0, &inventory.Asset{}, &inventory.Config{
+		Options:     map[string]string{},
+		Credentials: []*vault.Credential{cred},
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "missing required Jamf credentials")
+}
+
+func TestNewJamfConnection_MissingClientID(t *testing.T) {
+	_, err := NewJamfConnection(0, &inventory.Asset{}, &inventory.Config{
+		Options: map[string]string{
+			"instance_domain": "https://example.jamfcloud.com",
+		},
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "missing required Jamf credentials")
+}
+
+func TestJamfConnection_PlatformInfo(t *testing.T) {
+	conn := &JamfConnection{}
+	platform, err := conn.PlatformInfo()
+	require.NoError(t, err)
+	assert.Equal(t, "jamf", platform.Name)
+	assert.Equal(t, "Jamf Pro", platform.Title)
+	assert.Equal(t, "api", platform.Kind)
+	assert.Equal(t, "jamf", platform.Runtime)
+	assert.Equal(t, []string{"jamf"}, platform.Family)
+}
+
+func TestJamfConnection_Identifier(t *testing.T) {
+	conn := &JamfConnection{
+		Conf: &inventory.Config{
+			Options: map[string]string{
+				"instance_domain": "https://MyCompany.jamfcloud.com",
+			},
+		},
+	}
+	id := conn.Identifier()
+	assert.Equal(t, "//platformid.api.mondoo.app/runtime/jamf/https://mycompany.jamfcloud.com", id)
+}
+
+func TestJamfConnection_Name(t *testing.T) {
+	conn := &JamfConnection{}
+	assert.Equal(t, "jamf", conn.Name())
+}
+
+func TestJamfConnection_LocalUserAccountsCache(t *testing.T) {
+	conn := &JamfConnection{
+		localUserAccounts: make(map[string][]jamfpro.ComputerInventorySubsetLocalUserAccount),
+	}
+
+	// Cache should be empty initially
+	_, ok := conn.GetCachedLocalUserAccounts("123")
+	assert.False(t, ok)
+
+	// After caching, should return data
+	testAccounts := []jamfpro.ComputerInventorySubsetLocalUserAccount{
+		{UID: "501", Username: "admin", Admin: true},
+		{UID: "502", Username: "user1", Admin: false},
+	}
+	conn.CacheLocalUserAccounts("123", testAccounts)
+
+	cached, ok := conn.GetCachedLocalUserAccounts("123")
+	assert.True(t, ok)
+	require.Len(t, cached, 2)
+	assert.Equal(t, "admin", cached[0].Username)
+	assert.True(t, cached[0].Admin)
+	assert.Equal(t, "user1", cached[1].Username)
+	assert.False(t, cached[1].Admin)
+
+	// Different ID should miss cache
+	_, ok = conn.GetCachedLocalUserAccounts("456")
+	assert.False(t, ok)
+}

providers/jamf/go.mod

@@ -7,6 +7,7 @@ go 1.25.8
 require (
 	github.com/deploymenttheory/go-api-sdk-jamfpro v1.28.0
 	github.com/rs/zerolog v1.35.0
+	github.com/stretchr/testify v1.11.1
 	go.mondoo.com/mql/v13 v13.4.0
 )
 
@@ -60,6 +61,7 @@ require (
 	github.com/cockroachdb/redact v1.1.8 // indirect
 	github.com/cyphar/filepath-securejoin v0.6.1 // indirect
 	github.com/danieljoos/wincred v1.2.3 // indirect
+	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/deploymenttheory/go-api-http-client v0.4.1 // indirect
 	github.com/deploymenttheory/go-api-http-client-integrations v0.0.13 // indirect
 	github.com/dvsekhvalnov/jose2go v1.8.0 // indirect
@@ -117,6 +119,7 @@ require (
 	github.com/pjbgf/sha1cd v0.5.0 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
+	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/rivo/uniseg v0.4.7 // indirect
 	github.com/rogpeppe/go-internal v1.14.1 // indirect
 	github.com/ryanuber/go-glob v1.0.0 // indirect
@@ -157,6 +160,7 @@ require (
 	google.golang.org/grpc v1.80.0 // indirect
 	google.golang.org/protobuf v1.36.11 // indirect
 	gopkg.in/warnings.v0 v0.1.2 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 	moul.io/http2curl v1.0.0 // indirect
 	sigs.k8s.io/yaml v1.6.0 // indirect
 )

providers/jamf/provider/provider.go

@@ -36,47 +36,22 @@ func (s *Service) ParseCLI(req *plugin.ParseCLIReq) (*plugin.ParseCLIRes, error)
 		flags = map[string]*llx.Primitive{}
 	}
 
-	conf := &inventory.Config{
-		Type:    req.Connector,
-		Options: map[string]string{},
-	}
-
-	// CLI flags for credentials
-	var clientId, clientSecret, instanceDomain string
-
-	if v, ok := flags["client-id"]; ok && len(v.Value) != 0 {
-		clientId = string(v.Value)
-		conf.Options["client_id"] = clientId
-	}
-	if v, ok := flags["client-secret"]; ok && len(v.Value) != 0 {
-		clientSecret = string(v.Value)
-		conf.Options["client_secret"] = clientSecret
-	}
-	if v, ok := flags["instance-domain"]; ok && len(v.Value) != 0 {
-		instanceDomain = string(v.Value)
-		conf.Options["instance_domain"] = instanceDomain
-	}
+	// Resolve credentials: flags take precedence over env vars
+	clientId := flagOrEnv(flags, "client-id", "CLIENT_ID")
+	clientSecret := flagOrEnv(flags, "client-secret", "CLIENT_SECRET")
+	instanceDomain := flagOrEnv(flags, "instance-domain", "INSTANCE_DOMAIN")
 
-	// Fallback to env if flags not provided
-	if clientId == "" {
-		clientId = os.Getenv("CLIENT_ID")
-	}
-	if clientSecret == "" {
-		clientSecret = os.Getenv("CLIENT_SECRET")
-	}
-	if instanceDomain == "" {
-		instanceDomain = os.Getenv("INSTANCE_DOMAIN")
+	conf := &inventory.Config{
+		Type: req.Connector,
+		Options: map[string]string{
+			"instance_domain": instanceDomain,
+		},
 	}
 
-	// Attach as credentials if available
 	if clientId != "" || clientSecret != "" {
 		conf.Credentials = append(conf.Credentials, vault.NewPasswordCredential(clientId, clientSecret))
 	}
 
-	if instanceDomain != "" && conf.Options["instance_domain"] == "" {
-		conf.Options["instance_domain"] = instanceDomain
-	}
-
 	asset := &inventory.Asset{
 		Name:        "Jamf",
 		Connections: []*inventory.Config{conf},
@@ -85,6 +60,13 @@ func (s *Service) ParseCLI(req *plugin.ParseCLIReq) (*plugin.ParseCLIRes, error)
 	return &plugin.ParseCLIRes{Asset: asset}, nil
 }
 
+func flagOrEnv(flags map[string]*llx.Primitive, flagName, envName string) string {
+	if v, ok := flags[flagName]; ok && len(v.Value) != 0 {
+		return string(v.Value)
+	}
+	return os.Getenv(envName)
+}
+
 func (s *Service) Connect(req *plugin.ConnectReq, callback plugin.ProviderCallback) (*plugin.ConnectRes, error) {
 	if req == nil || req.Asset == nil {
 		return nil, errors.New("no connection data provided")

providers/jamf/provider/provider_test.go

@@ -0,0 +1,117 @@
+// Copyright Mondoo, Inc. 2024, 2026
+// SPDX-License-Identifier: BUSL-1.1
+
+package provider
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.mondoo.com/mql/v13/llx"
+	"go.mondoo.com/mql/v13/providers-sdk/v1/plugin"
+)
+
+func TestParseCLI_WithFlags(t *testing.T) {
+	s := Init()
+
+	res, err := s.ParseCLI(&plugin.ParseCLIReq{
+		Connector: "jamf",
+		Flags: map[string]*llx.Primitive{
+			"client-id":       {Value: []byte("my-client-id")},
+			"client-secret":   {Value: []byte("my-client-secret")},
+			"instance-domain": {Value: []byte("https://example.jamfcloud.com")},
+		},
+	})
+	require.NoError(t, err)
+	require.NotNil(t, res.Asset)
+
+	conf := res.Asset.Connections[0]
+	assert.Equal(t, "jamf", conf.Type)
+	assert.Equal(t, "https://example.jamfcloud.com", conf.Options["instance_domain"])
+
+	// Credentials should be set
+	require.Len(t, conf.Credentials, 1)
+	assert.Equal(t, "my-client-id", conf.Credentials[0].User)
+	assert.Equal(t, "my-client-secret", string(conf.Credentials[0].Secret))
+
+	// Secrets should NOT be in Options
+	_, hasClientId := conf.Options["client_id"]
+	_, hasClientSecret := conf.Options["client_secret"]
+	assert.False(t, hasClientId, "client_id should not be in Options")
+	assert.False(t, hasClientSecret, "client_secret should not be in Options")
+}
+
+func TestParseCLI_EmptyFlags(t *testing.T) {
+	s := Init()
+
+	res, err := s.ParseCLI(&plugin.ParseCLIReq{
+		Connector: "jamf",
+		Flags:     nil,
+	})
+	require.NoError(t, err)
+	require.NotNil(t, res.Asset)
+
+	conf := res.Asset.Connections[0]
+	assert.Equal(t, "jamf", conf.Type)
+	assert.Empty(t, conf.Credentials)
+}
+
+func TestParseCLI_PartialFlags(t *testing.T) {
+	s := Init()
+
+	res, err := s.ParseCLI(&plugin.ParseCLIReq{
+		Connector: "jamf",
+		Flags: map[string]*llx.Primitive{
+			"instance-domain": {Value: []byte("https://example.jamfcloud.com")},
+		},
+	})
+	require.NoError(t, err)
+	require.NotNil(t, res.Asset)
+
+	conf := res.Asset.Connections[0]
+	assert.Equal(t, "https://example.jamfcloud.com", conf.Options["instance_domain"])
+	assert.Empty(t, conf.Credentials, "no credentials when only domain is provided")
+}
+
+func TestParseCLI_EnvVarFallback(t *testing.T) {
+	t.Setenv("CLIENT_ID", "env-client-id")
+	t.Setenv("CLIENT_SECRET", "env-client-secret")
+	t.Setenv("INSTANCE_DOMAIN", "https://env.jamfcloud.com")
+
+	s := Init()
+
+	res, err := s.ParseCLI(&plugin.ParseCLIReq{
+		Connector: "jamf",
+		Flags:     map[string]*llx.Primitive{},
+	})
+	require.NoError(t, err)
+	require.NotNil(t, res.Asset)
+
+	conf := res.Asset.Connections[0]
+	assert.Equal(t, "https://env.jamfcloud.com", conf.Options["instance_domain"])
+	require.Len(t, conf.Credentials, 1)
+	assert.Equal(t, "env-client-id", conf.Credentials[0].User)
+	assert.Equal(t, "env-client-secret", string(conf.Credentials[0].Secret))
+}
+
+func TestParseCLI_FlagsTakePrecedenceOverEnv(t *testing.T) {
+	t.Setenv("CLIENT_ID", "env-client-id")
+	t.Setenv("INSTANCE_DOMAIN", "https://env.jamfcloud.com")
+
+	s := Init()
+
+	res, err := s.ParseCLI(&plugin.ParseCLIReq{
+		Connector: "jamf",
+		Flags: map[string]*llx.Primitive{
+			"client-id":       {Value: []byte("flag-client-id")},
+			"instance-domain": {Value: []byte("https://flag.jamfcloud.com")},
+		},
+	})
+	require.NoError(t, err)
+
+	conf := res.Asset.Connections[0]
+	assert.Equal(t, "https://flag.jamfcloud.com", conf.Options["instance_domain"])
+	require.Len(t, conf.Credentials, 1)
+	assert.Equal(t, "flag-client-id", conf.Credentials[0].User)
+}