From f2e305a56df7a28d33e279c9f6bad77925931904 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Mon, 9 Nov 2020 20:19:38 -0600 Subject: [PATCH] Fix missing authentication for redfish dell client Change-Id: Iffd26171b75ce10dd15d66583b6bda249156c34d Closes: #396 --- pkg/remote/redfish/client.go | 26 +++------ pkg/remote/redfish/client_test.go | 56 +++++++++---------- pkg/remote/redfish/utils.go | 14 +++++ pkg/remote/redfish/vendors/dell/client.go | 13 +++-- .../redfish/vendors/dell/client_test.go | 5 +- 5 files changed, 61 insertions(+), 53 deletions(-) diff --git a/pkg/remote/redfish/client.go b/pkg/remote/redfish/client.go index 8ce4d3ea9..07ffc76ba 100644 --- a/pkg/remote/redfish/client.go +++ b/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, diff --git a/pkg/remote/redfish/client_test.go b/pkg/remote/redfish/client_test.go index 06ebc09cc..13a05c157 100644 --- a/pkg/remote/redfish/client_test.go +++ b/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 diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index bff28afa6..8e08e3173 100644 --- a/pkg/remote/redfish/utils.go +++ b/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 { diff --git a/pkg/remote/redfish/vendors/dell/client.go b/pkg/remote/redfish/vendors/dell/client.go index 8a1d2878a..59446e430 100644 --- a/pkg/remote/redfish/vendors/dell/client.go +++ b/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 auth, ok := ctx.Value(redfishClient.ContextBasicAuth).(redfishClient.BasicAuth); ok { - req.SetBasicAuth(auth.UserName, auth.Password) + if len(c.password+c.username) != 0 { + req.SetBasicAuth(c.username, c.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 } diff --git a/pkg/remote/redfish/vendors/dell/client_test.go b/pkg/remote/redfish/vendors/dell/client_test.go index f3aaa83a5..29377b157 100644 --- a/pkg/remote/redfish/vendors/dell/client_test.go +++ b/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{},