Fix missing authentication for redfish dell client

Change-Id: Iffd26171b75ce10dd15d66583b6bda249156c34d
Closes: #396

pkg/remote/redfish/client.go

@@ -63,7 +63,7 @@ func (c *Client) SystemRebootDelay() int {
 
 // EjectVirtualMedia ejects a virtual media device attached to a host.
 func (c *Client) EjectVirtualMedia(ctx context.Context) error {
-	ctx = c.setAuth(ctx)
+	ctx = SetAuth(ctx, c.username, c.password)
 	waitForEjectMedia := func(managerID string, mediaID string) error {
 		for retry := 0; retry < c.systemActionRetries; retry++ {
 			vMediaMgr, httpResp, err := c.RedfishAPI.GetManagerVirtualMedia(ctx, managerID, mediaID)
@@ -120,7 +120,7 @@ func (c *Client) EjectVirtualMedia(ctx context.Context) error {
 // RebootSystem power cycles a host by sending a shutdown signal followed by a power on signal.
 func (c *Client) RebootSystem(ctx context.Context) error {
 	log.Debugf("Rebooting node '%s': powering off.", c.nodeID)
-	ctx = c.setAuth(ctx)
+	ctx = SetAuth(ctx, c.username, c.password)
 	resetReq := redfishClient.ResetRequestBody{}
 
 	// Send PowerOff request
@@ -153,7 +153,7 @@ func (c *Client) RebootSystem(ctx context.Context) error {
 // SetBootSourceByType sets the boot source of the ephemeral node to one that's compatible with the boot
 // source type.
 func (c *Client) SetBootSourceByType(ctx context.Context) error {
-	ctx = c.setAuth(ctx)
+	ctx = SetAuth(ctx, c.username, c.password)
 	_, vMediaType, err := GetVirtualMediaID(ctx, c.RedfishAPI, c.nodeID)
 	if err != nil {
 		return err
@@ -189,7 +189,7 @@ func (c *Client) SetBootSourceByType(ctx context.Context) error {
 // SetVirtualMedia injects a virtual media device to an established virtual media ID. This assumes that isoPath is
 // accessible to the redfish server and virtualMedia device is either of type CD or DVD.
 func (c *Client) SetVirtualMedia(ctx context.Context, isoPath string) error {
-	ctx = c.setAuth(ctx)
+	ctx = SetAuth(ctx, c.username, c.password)
 	log.Debugf("Inserting virtual media '%s'.", isoPath)
 	// Eject all previously-inserted media
 	if err := c.EjectVirtualMedia(ctx); err != nil {
@@ -223,7 +223,7 @@ func (c *Client) SetVirtualMedia(ctx context.Context, isoPath string) error {
 
 // SystemPowerOff shuts down a host.
 func (c *Client) SystemPowerOff(ctx context.Context) error {
-	ctx = c.setAuth(ctx)
+	ctx = SetAuth(ctx, c.username, c.password)
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
 
@@ -237,7 +237,7 @@ func (c *Client) SystemPowerOff(ctx context.Context) error {
 
 // SystemPowerOn powers on a host.
 func (c *Client) SystemPowerOn(ctx context.Context) error {
-	ctx = c.setAuth(ctx)
+	ctx = SetAuth(ctx, c.username, c.password)
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_ON
 
@@ -251,7 +251,7 @@ func (c *Client) SystemPowerOn(ctx context.Context) error {
 
 // SystemPowerStatus retrieves the power status of a host as a human-readable string.
 func (c *Client) SystemPowerStatus(ctx context.Context) (power.Status, error) {
-	ctx = c.setAuth(ctx)
+	ctx = SetAuth(ctx, c.username, c.password)
 	computerSystem, httpResp, err := c.RedfishAPI.GetSystem(ctx, c.nodeID)
 	if err = ScreenRedfishError(httpResp, err); err != nil {
 		return power.StatusUnknown, err
@@ -271,18 +271,6 @@ func (c *Client) SystemPowerStatus(ctx context.Context) (power.Status, error) {
 	}
 }
 
-func (c *Client) setAuth(ctx context.Context) context.Context {
-	authValue := redfishClient.BasicAuth{UserName: c.username, Password: c.password}
-	if ctx.Value(redfishClient.ContextBasicAuth) == authValue {
-		return ctx
-	}
-	return context.WithValue(
-		ctx,
-		redfishClient.ContextBasicAuth,
-		redfishClient.BasicAuth{UserName: c.username, Password: c.password},
-	)
-}
-
 // NewClient returns a client with the capability to make Redfish requests.
 func NewClient(redfishURL string,
 	insecure bool,

pkg/remote/redfish/client_test.go

@@ -82,7 +82,7 @@ func TestEjectVirtualMedia(t *testing.T) {
 	assert.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	// Mark CD and DVD test media as inserted
 	inserted := true
@@ -137,7 +137,7 @@ func TestEjectVirtualMediaRetriesExceeded(t *testing.T) {
 
 	client.nodeID = nodeID
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	// Mark test media as inserted
 	inserted := true
@@ -176,7 +176,7 @@ func TestRebootSystem(t *testing.T) {
 	assert.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	// Mock redfish shutdown and status requests
 	resetReq := redfishClient.ResetRequestBody{}
@@ -211,7 +211,7 @@ func TestRebootSystemShutdownError(t *testing.T) {
 	assert.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
@@ -238,7 +238,7 @@ func TestRebootSystemStartupError(t *testing.T) {
 	assert.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
@@ -276,7 +276,7 @@ func TestRebootSystemTimeout(t *testing.T) {
 	assert.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
@@ -305,7 +305,7 @@ func TestSetBootSourceByTypeGetSystemError(t *testing.T) {
 	assert.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	// Mock redfish get system request
 	m.On("GetSystem", ctx, client.NodeID()).Times(1).Return(redfishClient.ComputerSystem{},
@@ -328,7 +328,7 @@ func TestSetBootSourceByTypeSetSystemError(t *testing.T) {
 	assert.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	httpResp := &http.Response{StatusCode: 200}
 	m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil)
@@ -355,7 +355,7 @@ func TestSetBootSourceByTypeBootSourceUnavailable(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	client.nodeID = nodeID
 
 	invalidSystem := testutil.GetTestSystem()
@@ -390,7 +390,7 @@ func TestSetVirtualMediaEjectExistingMedia(t *testing.T) {
 
 	client.nodeID = nodeID
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	// Mark test media as inserted
 	inserted := true
@@ -436,7 +436,7 @@ func TestSetVirtualMediaEjectExistingMediaFailure(t *testing.T) {
 
 	client.nodeID = nodeID
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	// Mark test media as inserted
 	inserted := true
@@ -471,7 +471,7 @@ func TestSetVirtualMediaGetSystemError(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	client.nodeID = nodeID
 
 	// Mock redfish get system request
@@ -494,7 +494,7 @@ func TestSetVirtualMediaInsertVirtualMediaError(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	client.nodeID = nodeID
 
 	httpResp := &http.Response{StatusCode: 200}
@@ -530,7 +530,7 @@ func TestSystemPowerOff(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return(
 		redfishClient.RedfishError{},
@@ -562,7 +562,7 @@ func TestSystemPowerOffResetSystemError(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return(
 		redfishClient.RedfishError{},
@@ -586,7 +586,7 @@ func TestSystemPowerOn(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return(
 		redfishClient.RedfishError{},
@@ -618,7 +618,7 @@ func TestSystemPowerOnResetSystemError(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return(
 		redfishClient.RedfishError{},
@@ -642,7 +642,7 @@ func TestSystemPowerStatusUnknown(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	var unknownState redfishClient.PowerState = "unknown"
 	m.On("GetSystem", ctx, client.nodeID).Return(
@@ -665,7 +665,7 @@ func TestSystemPowerStatusOn(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	require.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	client.nodeID = nodeID
 
 	m.On("GetSystem", ctx, client.nodeID).Return(
@@ -689,7 +689,7 @@ func TestSystemPowerStatusOff(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	m.On("GetSystem", ctx, client.nodeID).Return(
 		redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_OFF},
@@ -712,7 +712,7 @@ func TestSystemPowerStatusPoweringOn(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	m.On("GetSystem", ctx, client.nodeID).Return(
 		redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_POWERING_ON},
@@ -735,7 +735,7 @@ func TestSystemPowerStatusPoweringOff(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	m.On("GetSystem", ctx, client.nodeID).Return(
 		redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_POWERING_OFF},
@@ -758,7 +758,7 @@ func TestSystemPowerStatusGetSystemError(t *testing.T) {
 	require.NoError(t, err)
 
 	client.nodeID = nodeID
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 
 	m.On("GetSystem", ctx, client.nodeID).Return(
 		redfishClient.ComputerSystem{},
@@ -778,7 +778,7 @@ func TestWaitForPowerStateGetSystemFailed(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
 
@@ -802,7 +802,7 @@ func TestWaitForPowerStateNoRetries(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
 
@@ -828,7 +828,7 @@ func TestWaitForPowerStateWithRetries(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
 
@@ -859,7 +859,7 @@ func TestWaitForPowerStateRetriesExceeded(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
 
@@ -890,7 +890,7 @@ func TestWaitForPowerStateDifferentPowerState(t *testing.T) {
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
 
-	ctx := client.setAuth(context.Background())
+	ctx := SetAuth(context.Background(), "", "")
 	resetReq := redfishClient.ResetRequestBody{}
 	resetReq.ResetType = redfishClient.RESETTYPE_FORCE_ON
 

pkg/remote/redfish/utils.go

@@ -231,6 +231,20 @@ func ScreenRedfishError(httpResp *http.Response, clientErr error) error {
 	return finalError
 }
 
+// SetAuth allows to set username and password to given context so that redfish client can
+// authenticate against redfish server
+func SetAuth(ctx context.Context, username string, password string) context.Context {
+	authValue := redfishClient.BasicAuth{UserName: username, Password: password}
+	if ctx.Value(redfishClient.ContextBasicAuth) == authValue {
+		return ctx
+	}
+	return context.WithValue(
+		ctx,
+		redfishClient.ContextBasicAuth,
+		redfishClient.BasicAuth{UserName: username, Password: password},
+	)
+}
+
 func getManagerID(ctx context.Context, api redfishAPI.RedfishAPI, systemID string) (string, error) {
 	system, _, err := api.GetSystem(ctx, systemID)
 	if err != nil {

pkg/remote/redfish/vendors/dell/client.go

@@ -50,6 +50,8 @@ const (
 // Client is a wrapper around the standard airshipctl Redfish client. This allows vendor specific Redfish clients to
 // override methods without duplicating the entire client.
 type Client struct {
+	username string
+	password string
 	redfish.Client
 	RedfishAPI redfishAPI.RedfishAPI
 	RedfishCFG *redfishClient.Configuration
@@ -73,7 +75,9 @@ type iDRACAPIExtendedInfo struct {
 // SetBootSourceByType sets the boot source of the ephemeral node to a virtual CD, "VCD-DVD".
 func (c *Client) SetBootSourceByType(ctx context.Context) error {
 	log.Debug("Setting boot device to 'VCD-DVD'.")
-	managerID, err := redfish.GetManagerID(ctx, c.RedfishAPI, c.NodeID())
+	managerID, err := redfish.GetManagerID(
+		redfish.SetAuth(ctx, c.username, c.password),
+		c.RedfishAPI, c.NodeID())
 	if err != nil {
 		log.Debugf("Failed to retrieve manager ID for node '%s'.", c.NodeID())
 		return err
@@ -90,9 +94,8 @@ func (c *Client) SetBootSourceByType(ctx context.Context) error {
 
 	req.Header.Add("Content-Type", "application/json")
 	req.Header.Add("Accept", "application/json")
-
+	if len(c.password+c.username) != 0 {
-	if auth, ok := ctx.Value(redfishClient.ContextBasicAuth).(redfishClient.BasicAuth); ok {
+		req.SetBasicAuth(c.username, c.password)
-		req.SetBasicAuth(auth.UserName, auth.Password)
 	}
 
 	httpResp, err := c.RedfishCFG.HTTPClient.Do(req)
@@ -137,7 +140,7 @@ func NewClient(redfishURL string,
 		return nil, err
 	}
 
-	c := &Client{*genericClient, genericClient.RedfishAPI, genericClient.RedfishCFG}
+	c := &Client{username, password, *genericClient, genericClient.RedfishAPI, genericClient.RedfishCFG}
 
 	return c, nil
 }

pkg/remote/redfish/vendors/dell/client_test.go

@@ -23,6 +23,8 @@ import (
 
 	redfishMocks "opendev.org/airship/go-redfish/api/mocks"
 	redfishClient "opendev.org/airship/go-redfish/client"
+
+	"opendev.org/airship/airshipctl/pkg/remote/redfish"
 )
 
 const (
@@ -50,7 +52,8 @@ func TestSetBootSourceByTypeGetSystemError(t *testing.T) {
 
 	client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay)
 	assert.NoError(t, err)
-	ctx := context.Background()
+
+	ctx := redfish.SetAuth(context.Background(), "", "")
 
 	// Mock redfish get system request
 	m.On("GetSystem", ctx, client.NodeID()).Times(1).Return(redfishClient.ComputerSystem{},