fix(dell): set Secure Boot via the SecureBoot BIOS attribute

internal/redfishwrapper/bios.go

@@ -65,15 +65,22 @@ func (c *Client) GetBiosConfiguration(ctx context.Context) (biosConfig map[strin
 
 // SetBiosConfiguration applies the given BIOS configuration attributes, to take effect on the next reset.
 func (c *Client) SetBiosConfiguration(ctx context.Context, biosConfig map[string]string) (err error) {
-	sys, err := c.System()
-	if err != nil {
-		return err
+	attrs := make(schemas.SettingsAttributes, len(biosConfig))
+	for attr, value := range biosConfig {
+		attrs[attr] = value
 	}
 
-	settingsAttributes := make(schemas.SettingsAttributes)
+	return c.ApplyBiosAttributes(ctx, attrs)
+}
 
-	for attr, value := range biosConfig {
-		settingsAttributes[attr] = value
+// ApplyBiosAttributes applies the given BIOS attributes, to take effect on the next reset. It is
+// SetBiosConfiguration for callers that hold attribute values in their native JSON type (bool,
+// number, string) rather than as strings, such as a caller resubmitting attributes it read back
+// from the BMC, where stringifying a value could be rejected by a strict attribute registry.
+func (c *Client) ApplyBiosAttributes(ctx context.Context, attrs schemas.SettingsAttributes) error {
+	sys, err := c.System()
+	if err != nil {
+		return err
 	}
 
 	if !c.compatibleOdataID(sys.ODataID, knownSystemsOdataIDs) {
@@ -86,14 +93,14 @@ func (c *Client) SetBiosConfiguration(ctx context.Context, biosConfig map[string
 	}
 
 	// TODO(jwb) We should handle passing different apply times here
-	err = bios.UpdateBiosAttributesApplyAt(settingsAttributes, schemas.OnResetSettingsApplyTime)
+	err = bios.UpdateBiosAttributesApplyAt(attrs, schemas.OnResetSettingsApplyTime)
 	if err != nil && rejectsSettingsApplyTime(err) {
 		// This BMC's Bios resource doesn't declare @Redfish.Settings.SupportedApplyTimes
 		// at all and rejects the @Redfish.SettingsApplyTime property outright, rather than
 		// ignoring it. Retry without an apply-time hint - the settings still go through the
 		// resource's separate Settings URI (@Redfish.Settings.SettingsObject), which by
 		// Redfish convention means they're staged rather than applied immediately.
-		return bios.UpdateBiosAttributes(settingsAttributes)
+		return bios.UpdateBiosAttributes(attrs)
 	}
 	return err
 }

internal/redfishwrapper/bios_test.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stmcginnis/gofish/schemas"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -257,3 +258,52 @@ func TestSetBiosConfiguration_WritesAttributeAlreadyMatchingAppliedState(t *test
 	require.Len(t, patchBodies, 1, "expected the write to be sent, not diffed away against applied state")
 	assert.Contains(t, patchBodies[0], `"BootModeSelect":"UEFI"`)
 }
+
+// TestApplyBiosAttributes_KeepsNativeValueTypes: unlike SetBiosConfiguration, which sends every
+// value as a string, ApplyBiosAttributes sends bool and number values as such, so a strict
+// attribute registry accepts them.
+func TestApplyBiosAttributes_KeepsNativeValueTypes(t *testing.T) {
+	var patched map[string]any
+
+	mux := http.NewServeMux()
+	mux.HandleFunc("/redfish/v1/", endpointFunc(t, "/dell/serviceroot.json"))
+	mux.HandleFunc("/redfish/v1/Systems", endpointFunc(t, "/dell/systems.json"))
+	mux.HandleFunc("/redfish/v1/Systems/System.Embedded.1", endpointFunc(t, "/dell/system.embedded.1.json"))
+	mux.HandleFunc("/redfish/v1/Systems/System.Embedded.1/Bios", func(w http.ResponseWriter, r *http.Request) {
+		switch r.Method {
+		case http.MethodGet:
+			_, _ = w.Write([]byte(biosWithoutSettingsApplyTimes))
+		case http.MethodPatch:
+			body, err := io.ReadAll(r.Body)
+			require.NoError(t, err)
+			require.NoError(t, json.Unmarshal(body, &patched))
+			w.WriteHeader(http.StatusNoContent)
+		default:
+			w.WriteHeader(http.StatusMethodNotAllowed)
+		}
+	})
+
+	server := httptest.NewTLSServer(mux)
+	defer server.Close()
+
+	parsedURL, err := url.Parse(server.URL)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "", WithBasicAuthEnabled(true))
+	require.NoError(t, client.Open(ctx))
+	defer client.Close(ctx)
+
+	err = client.ApplyBiosAttributes(ctx, schemas.SettingsAttributes{
+		"Flag":  true,
+		"Count": json.Number("42"),
+		"Name":  "x",
+	})
+	require.NoError(t, err)
+
+	attrs, ok := patched["Attributes"].(map[string]any)
+	require.True(t, ok, "expected an Attributes object in the PATCH body, got %v", patched)
+	assert.Equal(t, true, attrs["Flag"])
+	assert.Equal(t, float64(42), attrs["Count"])
+	assert.Equal(t, "x", attrs["Name"])
+}

internal/redfishwrapper/client.go

@@ -3,6 +3,7 @@ package redfishwrapper
 import (
 	"context"
 	"crypto/x509"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
@@ -336,6 +337,75 @@ func (c *Client) Tasks(ctx context.Context) ([]*schemas.Task, error) {
 	return ts.Tasks()
 }
 
+// Jobs returns the jobs currently tracked by the redfish job service. It returns an error if the
+// BMC has no JobService.
+//
+// The collection is requested expanded, which a BMC that supports $expand answers in one request.
+// Reading it job by job takes one request per job, and a used iDRAC holds well over a hundred of
+// them. A BMC that does not expand leaves the members as links, and each is then read on its own.
+func (c *Client) Jobs(ctx context.Context) ([]*schemas.Job, error) {
+	js, err := c.client.Service.JobService()
+	if err != nil {
+		return nil, err
+	}
+	if js == nil {
+		return nil, errors.New("BMC has no JobService")
+	}
+
+	if jobs, ok := c.expandedJobs(js); ok {
+		return jobs, nil
+	}
+
+	return js.Jobs()
+}
+
+// expandedJobs reads the job collection with $expand. It reports false if the BMC did not return
+// every member expanded, so that the caller can read the jobs one by one instead.
+func (c *Client) expandedJobs(js *schemas.JobService) ([]*schemas.Job, bool) {
+	var service struct {
+		Jobs struct {
+			ODataID string `json:"@odata.id"`
+		} `json:"Jobs"`
+	}
+	if json.Unmarshal(js.RawData, &service) != nil || service.Jobs.ODataID == "" {
+		return nil, false
+	}
+
+	resp, err := c.Get(service.Jobs.ODataID + "?$expand=*($levels=1)")
+	if err != nil {
+		return nil, false
+	}
+	defer func() { _ = resp.Body.Close() }()
+	if resp.StatusCode != http.StatusOK {
+		return nil, false
+	}
+
+	var collection struct {
+		Members  []json.RawMessage `json:"Members"`
+		Count    int               `json:"Members@odata.count"`
+		NextLink string            `json:"Members@odata.nextLink"`
+	}
+	if json.NewDecoder(resp.Body).Decode(&collection) != nil {
+		return nil, false
+	}
+
+	// A paginated collection, or one that reports more members than it returned, is incomplete.
+	if collection.NextLink != "" || (collection.Count != 0 && collection.Count != len(collection.Members)) {
+		return nil, false
+	}
+
+	jobs := make([]*schemas.Job, 0, len(collection.Members))
+	for _, member := range collection.Members {
+		var job schemas.Job
+		if json.Unmarshal(member, &job) != nil || job.JobState == "" || job.ODataID == "" {
+			return nil, false
+		}
+		jobs = append(jobs, &job)
+	}
+
+	return jobs, true
+}
+
 // ManagerOdataID returns the Odata ID of the first available Manager.
 func (c *Client) ManagerOdataID(ctx context.Context) (string, error) {
 	managers, err := c.client.Service.Managers()

internal/redfishwrapper/client_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/stmcginnis/gofish/schemas"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
 	"github.com/bmc-toolbox/bmclib/v2/internal/httpclient"
@@ -414,3 +415,183 @@ func TestOpenBoundsConnectByContext(t *testing.T) {
 	assert.Error(t, err, "expected the connect to fail once it outlives the ctx deadline")
 	assert.Less(t, elapsed, serverDelay, "expected Open to fail before the server even responds, bounded by ctx rather than by the much longer configured timeout")
 }
+
+func TestJobs(t *testing.T) {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/redfish/v1/", endpointFunc(t, "/dell/serviceroot.json"))
+	mux.HandleFunc("/redfish/v1/JobService", func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(`{"Jobs": {"@odata.id": "/redfish/v1/JobService/Jobs"}}`))
+	})
+	mux.HandleFunc("/redfish/v1/JobService/Jobs", func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(`{"Members": [{"@odata.id": "/redfish/v1/JobService/Jobs/JID_1"}], "Members@odata.count": 1}`))
+	})
+	mux.HandleFunc("/redfish/v1/JobService/Jobs/JID_1", func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(`{"Id": "JID_1", "@odata.id": "/redfish/v1/JobService/Jobs/JID_1", "Name": "ConfigBIOS:BIOS.Setup.1-1", "JobState": "Scheduled"}`))
+	})
+
+	server := httptest.NewTLSServer(mux)
+	defer server.Close()
+
+	parsedURL, err := url.Parse(server.URL)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "", WithBasicAuthEnabled(true))
+	require.NoError(t, client.Open(ctx))
+	defer client.Close(ctx)
+
+	jobs, err := client.Jobs(ctx)
+	require.NoError(t, err)
+	require.Len(t, jobs, 1)
+	assert.Equal(t, "JID_1", jobs[0].ID)
+	assert.Equal(t, "ConfigBIOS:BIOS.Setup.1-1", jobs[0].Name)
+	assert.Equal(t, "Scheduled", string(jobs[0].JobState))
+}
+
+func TestJobsWithoutJobService(t *testing.T) {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/redfish/v1/", endpointFunc(t, "/dell/serviceroot.json"))
+	mux.HandleFunc("/redfish/v1/JobService", func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	})
+
+	server := httptest.NewTLSServer(mux)
+	defer server.Close()
+
+	parsedURL, err := url.Parse(server.URL)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "", WithBasicAuthEnabled(true))
+	require.NoError(t, client.Open(ctx))
+	defer client.Close(ctx)
+
+	_, err = client.Jobs(ctx)
+	assert.Error(t, err)
+}
+
+// jobServiceMux serves a JobService with one job. If expand is true the collection answers
+// $expand with the members inline, and reading the job on its own then fails the test; otherwise
+// the collection answers with links only, whatever it was asked for.
+func jobServiceMux(t *testing.T, expand bool) *http.ServeMux {
+	t.Helper()
+
+	const job = `{"Id": "JID_1", "@odata.id": "/redfish/v1/JobService/Jobs/JID_1", "Name": "ConfigBIOS:BIOS.Setup.1-1", "JobState": "Scheduled"}`
+
+	mux := http.NewServeMux()
+	mux.HandleFunc("/redfish/v1/", endpointFunc(t, "/dell/serviceroot.json"))
+	mux.HandleFunc("/redfish/v1/JobService", func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(`{"Jobs": {"@odata.id": "/redfish/v1/JobService/Jobs"}}`))
+	})
+	mux.HandleFunc("/redfish/v1/JobService/Jobs", func(w http.ResponseWriter, r *http.Request) {
+		if expand && r.URL.Query().Has("$expand") {
+			_, _ = w.Write([]byte(`{"Members": [` + job + `], "Members@odata.count": 1}`))
+			return
+		}
+		if r.URL.Query().Has("$expand") && !expand {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		_, _ = w.Write([]byte(`{"Members": [{"@odata.id": "/redfish/v1/JobService/Jobs/JID_1"}], "Members@odata.count": 1}`))
+	})
+	mux.HandleFunc("/redfish/v1/JobService/Jobs/JID_1", func(w http.ResponseWriter, r *http.Request) {
+		if expand {
+			t.Error("job read on its own although the collection was returned expanded")
+		}
+		_, _ = w.Write([]byte(job))
+	})
+
+	return mux
+}
+
+func TestJobsReadsTheExpandedCollectionInOneRequest(t *testing.T) {
+	server := httptest.NewTLSServer(jobServiceMux(t, true))
+	defer server.Close()
+
+	parsedURL, err := url.Parse(server.URL)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "", WithBasicAuthEnabled(true))
+	require.NoError(t, client.Open(ctx))
+	defer client.Close(ctx)
+
+	jobs, err := client.Jobs(ctx)
+	require.NoError(t, err)
+	require.Len(t, jobs, 1)
+	assert.Equal(t, "JID_1", jobs[0].ID)
+	assert.Equal(t, "ConfigBIOS:BIOS.Setup.1-1", jobs[0].Name)
+	assert.Equal(t, "Scheduled", string(jobs[0].JobState))
+	assert.Equal(t, "/redfish/v1/JobService/Jobs/JID_1", jobs[0].ODataID, "expected the job's own URI, needed to act on it")
+}
+
+func TestJobsFallsBackToReadingEachJobWhenExpandIsRefused(t *testing.T) {
+	server := httptest.NewTLSServer(jobServiceMux(t, false))
+	defer server.Close()
+
+	parsedURL, err := url.Parse(server.URL)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "", WithBasicAuthEnabled(true))
+	require.NoError(t, client.Open(ctx))
+	defer client.Close(ctx)
+
+	jobs, err := client.Jobs(ctx)
+	require.NoError(t, err)
+	require.Len(t, jobs, 1)
+	assert.Equal(t, "JID_1", jobs[0].ID)
+}
+
+// TestJobsFallsBackWhenTheExpandedCollectionIsIncomplete: an expanded answer that is paginated,
+// reports more members than it holds, or has a member that cannot be acted on must not be taken
+// at face value, since a job missing from it would go unnoticed.
+func TestJobsFallsBackWhenTheExpandedCollectionIsIncomplete(t *testing.T) {
+	const job = `{"Id": "JID_1", "@odata.id": "/redfish/v1/JobService/Jobs/JID_1", "Name": "ConfigBIOS:BIOS.Setup.1-1", "JobState": "Scheduled"}`
+
+	tests := map[string]string{
+		"next link":          `{"Members": [` + job + `], "Members@odata.count": 1, "Members@odata.nextLink": "/redfish/v1/JobService/Jobs?$skip=1"}`,
+		"count mismatch":     `{"Members": [` + job + `], "Members@odata.count": 2}`,
+		"member without uri": `{"Members": [{"Id": "JID_1", "Name": "ConfigBIOS:BIOS.Setup.1-1", "JobState": "Scheduled"}], "Members@odata.count": 1}`,
+	}
+
+	for name, expanded := range tests {
+		t.Run(name, func(t *testing.T) {
+			var readOnOwn bool
+
+			mux := http.NewServeMux()
+			mux.HandleFunc("/redfish/v1/", endpointFunc(t, "/dell/serviceroot.json"))
+			mux.HandleFunc("/redfish/v1/JobService", func(w http.ResponseWriter, r *http.Request) {
+				_, _ = w.Write([]byte(`{"Jobs": {"@odata.id": "/redfish/v1/JobService/Jobs"}}`))
+			})
+			mux.HandleFunc("/redfish/v1/JobService/Jobs", func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Query().Has("$expand") {
+					_, _ = w.Write([]byte(expanded))
+					return
+				}
+				_, _ = w.Write([]byte(`{"Members": [{"@odata.id": "/redfish/v1/JobService/Jobs/JID_1"}], "Members@odata.count": 1}`))
+			})
+			mux.HandleFunc("/redfish/v1/JobService/Jobs/JID_1", func(w http.ResponseWriter, r *http.Request) {
+				readOnOwn = true
+				_, _ = w.Write([]byte(job))
+			})
+
+			server := httptest.NewTLSServer(mux)
+			defer server.Close()
+
+			parsedURL, err := url.Parse(server.URL)
+			require.NoError(t, err)
+
+			ctx := context.Background()
+			client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "", WithBasicAuthEnabled(true))
+			require.NoError(t, client.Open(ctx))
+			defer client.Close(ctx)
+
+			jobs, err := client.Jobs(ctx)
+			require.NoError(t, err)
+			require.Len(t, jobs, 1)
+			assert.Equal(t, "/redfish/v1/JobService/Jobs/JID_1", jobs[0].ODataID)
+			assert.True(t, readOnOwn, "expected the jobs to be read one by one instead")
+		})
+	}
+}