From f847c2eddeb2eaaabc4f89df7e050b8115f56396 Mon Sep 17 00:00:00 2001 From: manhtukhang Date: Wed, 24 Apr 2024 00:11:51 +0700 Subject: [PATCH 1/4] chore(source): refactored path config --- src/backend.go | 2 +- src/path_config.go | 100 ++++++++++++++++++++-------------------- src/path_config_test.go | 8 ++-- 3 files changed, 54 insertions(+), 56 deletions(-) diff --git a/src/backend.go b/src/backend.go index 843f742..bf87abb 100644 --- a/src/backend.go +++ b/src/backend.go @@ -48,7 +48,7 @@ func newBackend() *backend { Invalidate: b.invalidate, PathsSpecial: &logical.Paths{ - SealWrapStorage: []string{pathConfigAdmin}, + SealWrapStorage: []string{configAdminPath}, }, Paths: framework.PathAppend( []*framework.Path{ diff --git a/src/path_config.go b/src/path_config.go index e693a21..b9f23de 100644 --- a/src/path_config.go +++ b/src/path_config.go @@ -2,34 +2,13 @@ package nxr import ( "context" - "errors" - "fmt" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) const ( - // pathConfigHelpSynopsis summarizes the help text for the configuration - pathConfigHelpSynopsis = `Configure the Nexus Repository admin configuration.` - - // pathConfigHelpDescription describes the help text for the configuration - pathConfigHelpDescription = ` -The Nexus Repository secret backend requires credentials for managing user. - -You must create a username ("username" parameter) -and password ("password" parameter) -and specify the Nexus Repository address ("url" parameter) -for the API before using this secrets backend. - -An optional "insecure" parameter will enable bypassing -the TLS connection verification with Nexus Repository -(when server using self-signed certificate). - -An optional "timeout" is the maximum time (in second) -to wait before the request times out. -` - pathConfigAdmin = "config/admin" + configAdminPath = "config/admin" ) // adminConfig includes the minimum configuration @@ -46,11 +25,11 @@ type adminConfig struct { // endpoint for the backend. func pathConfig(b *backend) *framework.Path { return &framework.Path{ - Pattern: pathConfigAdmin, + Pattern: configAdminPath, Fields: map[string]*framework.FieldSchema{ "username": { Type: framework.TypeLowerCaseString, - Description: "The username to access Nexus Repository API", + Description: "The username to access Nexus Repository API.", Required: true, DisplayAttrs: &framework.DisplayAttributes{ Name: "Username", @@ -59,7 +38,7 @@ func pathConfig(b *backend) *framework.Path { }, "password": { Type: framework.TypeLowerCaseString, - Description: "The user's password to access Nexus Repository API", + Description: "The user's password to access Nexus Repository API.", Required: true, DisplayAttrs: &framework.DisplayAttributes{ Name: "Password", @@ -68,7 +47,7 @@ func pathConfig(b *backend) *framework.Path { }, "url": { Type: framework.TypeLowerCaseString, - Description: "The URL for the Nexus Repository API", + Description: "The URL for the Nexus Repository API.", Required: true, DisplayAttrs: &framework.DisplayAttributes{ Name: "URL", @@ -78,7 +57,7 @@ func pathConfig(b *backend) *framework.Path { "insecure": { Type: framework.TypeBool, Default: false, - Description: "Optional. Bypass certification verification for TLS connection with Nexus Repository API. Default to `false`", + Description: "Optional. Bypass certification verification for TLS connection with Nexus Repository API. Default to `false`.", DisplayAttrs: &framework.DisplayAttributes{ Name: "Insecure", Sensitive: false, @@ -87,7 +66,7 @@ func pathConfig(b *backend) *framework.Path { "timeout": { Type: framework.TypeInt, Default: 30, - Description: "Optional. Timeout for connection with Nexus Repository API. Default to `30` (second)", + Description: "Optional. Timeout for connection with Nexus Repository API. Default to `30` (second).", DisplayAttrs: &framework.DisplayAttributes{ Name: "Timeout", Sensitive: false, @@ -97,19 +76,19 @@ func pathConfig(b *backend) *framework.Path { Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ Callback: b.pathConfigRead, - Summary: "Examine the Nexus Repository admin configuration", + Summary: "Examine the Nexus Repository admin configuration.", }, logical.CreateOperation: &framework.PathOperation{ Callback: b.pathConfigWrite, - Summary: "Create the Nexus Repository admin configuration", + Summary: "Create the Nexus Repository admin configuration.", }, logical.UpdateOperation: &framework.PathOperation{ Callback: b.pathConfigWrite, - Summary: "Update (overwrite) the Nexus Repository admin configuration", + Summary: "Update (overwrite) the Nexus Repository admin configuration.", }, logical.DeleteOperation: &framework.PathOperation{ Callback: b.pathConfigDelete, - Summary: "Delete the Nexus Repository admin configuration", + Summary: "Delete the Nexus Repository admin configuration.", }, }, ExistenceCheck: b.pathConfigExistenceCheck, @@ -122,7 +101,7 @@ func pathConfig(b *backend) *framework.Path { func (b *backend) pathConfigExistenceCheck(ctx context.Context, req *logical.Request, data *framework.FieldData) (bool, error) { out, err := req.Storage.Get(ctx, req.Path) if err != nil { - return false, fmt.Errorf("existence check failed: %w", err) + return false, err } return out != nil, nil @@ -133,7 +112,7 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, data b.configMutex.Lock() defer b.configMutex.Unlock() - config, err := fetchAdminConfig(ctx, req.Storage) + config, err := b.fetchAdminConfig(ctx, req.Storage) if err != nil { return nil, err } @@ -156,35 +135,33 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, dat b.configMutex.Lock() defer b.configMutex.Unlock() - createOperation := (req.Operation == logical.CreateOperation) - - config, err := fetchAdminConfig(ctx, req.Storage) + config, err := b.fetchAdminConfig(ctx, req.Storage) if err != nil { return nil, err } if config == nil { - if !createOperation { - return nil, errors.New("admin configuration not found during update operation") - } - config = new(adminConfig) + config = &adminConfig{} } + createOperation := (req.Operation == logical.CreateOperation) + if username, ok := data.GetOk("username"); ok { config.Username = username.(string) } else if !ok && createOperation { - return nil, fmt.Errorf("missing username in admin configuration") + return logical.ErrorResponse("missing username in admin configuration"), nil } if url, ok := data.GetOk("url"); ok { config.URL = url.(string) + config.Password = "" // NOTE: clear password if URL changes, requires setting password and url together for security reasons } else if !ok && createOperation { - return nil, fmt.Errorf("missing url in admin configuration") + return logical.ErrorResponse("missing url in admin configuration"), nil } if password, ok := data.GetOk("password"); ok { config.Password = password.(string) } else if !ok && createOperation { - return nil, fmt.Errorf("missing password in admin configuration") + return logical.ErrorResponse("missing password in admin configuration"), nil } if insecure, ok := data.GetOk("insecure"); ok { @@ -195,7 +172,7 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, dat config.Timeout = timeout.(int) } - entry, err := logical.StorageEntryJSON(pathConfigAdmin, config) + entry, err := logical.StorageEntryJSON(configAdminPath, config) if err != nil { return nil, err } @@ -215,7 +192,7 @@ func (b *backend) pathConfigDelete(ctx context.Context, req *logical.Request, da b.configMutex.Lock() defer b.configMutex.Unlock() - config, err := fetchAdminConfig(ctx, req.Storage) + config, err := b.fetchAdminConfig(ctx, req.Storage) if err != nil { return nil, err } @@ -223,7 +200,7 @@ func (b *backend) pathConfigDelete(ctx context.Context, req *logical.Request, da return logical.ErrorResponse("admin configuration not found"), nil } - err = req.Storage.Delete(ctx, pathConfigAdmin) + err = req.Storage.Delete(ctx, configAdminPath) if err == nil { b.client = nil } @@ -231,8 +208,9 @@ func (b *backend) pathConfigDelete(ctx context.Context, req *logical.Request, da return nil, err } -func fetchAdminConfig(ctx context.Context, s logical.Storage) (*adminConfig, error) { - entry, err := s.Get(ctx, pathConfigAdmin) +// fetchAdminConfig fetches admin configuration for the backend +func (b *backend) fetchAdminConfig(ctx context.Context, s logical.Storage) (*adminConfig, error) { + entry, err := s.Get(ctx, configAdminPath) if err != nil { return nil, err } @@ -241,11 +219,31 @@ func fetchAdminConfig(ctx context.Context, s logical.Storage) (*adminConfig, err return nil, nil } - config := new(adminConfig) + config := &adminConfig{} if err := entry.DecodeJSON(&config); err != nil { - return nil, fmt.Errorf("error fetching admin configuration: %w", err) + return nil, err } // return the config, we are done return config, nil } + +const ( + pathConfigHelpSynopsis = `Configure the Nexus Repository admin configuration.` + + pathConfigHelpDescription = ` +The Nexus Repository secret backend requires credentials for managing user. + +You must create a username ("username" parameter) +and password ("password" parameter) +and specify the Nexus Repository address ("url" parameter) +for the API before using this secrets backend. + +An optional "insecure" parameter will enable bypassing +the TLS connection verification with Nexus Repository +(when server using self-signed certificate). + +An optional "timeout" parameter is the maximum time (in seconds) +to wait before the request to the API is timed out. +` +) diff --git a/src/path_config_test.go b/src/path_config_test.go index d1de462..edf8e71 100644 --- a/src/path_config_test.go +++ b/src/path_config_test.go @@ -99,7 +99,7 @@ func TestConfig(t *testing.T) { func testConfigDelete(b logical.Backend, s logical.Storage) error { resp, err := b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.DeleteOperation, - Path: pathConfigAdmin, + Path: configAdminPath, Storage: s, }) if err != nil { @@ -115,7 +115,7 @@ func testConfigDelete(b logical.Backend, s logical.Storage) error { func testConfigCreate(b logical.Backend, s logical.Storage, d map[string]interface{}) error { resp, err := b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.CreateOperation, - Path: pathConfigAdmin, + Path: configAdminPath, Data: d, Storage: s, }) @@ -132,7 +132,7 @@ func testConfigCreate(b logical.Backend, s logical.Storage, d map[string]interfa func testConfigUpdate(b logical.Backend, s logical.Storage, d map[string]interface{}) error { resp, err := b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, - Path: pathConfigAdmin, + Path: configAdminPath, Data: d, Storage: s, }) @@ -149,7 +149,7 @@ func testConfigUpdate(b logical.Backend, s logical.Storage, d map[string]interfa func testConfigRead(b logical.Backend, s logical.Storage, expected map[string]interface{}) error { resp, err := b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ReadOperation, - Path: pathConfigAdmin, + Path: configAdminPath, Storage: s, }) if err != nil { From abdfdb13aad20590255a235d5bbf7c12e8999c55 Mon Sep 17 00:00:00 2001 From: manhtukhang Date: Wed, 24 Apr 2024 17:27:01 +0700 Subject: [PATCH 2/4] fix: fixed wrong admin password type Signed-off-by: manhtukhang --- src/path_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/path_config.go b/src/path_config.go index b9f23de..2887aac 100644 --- a/src/path_config.go +++ b/src/path_config.go @@ -37,7 +37,7 @@ func pathConfig(b *backend) *framework.Path { }, }, "password": { - Type: framework.TypeLowerCaseString, + Type: framework.TypeString, Description: "The user's password to access Nexus Repository API.", Required: true, DisplayAttrs: &framework.DisplayAttributes{ From f0aa4fbe0a76a0dadacea56164a170717dc3204d Mon Sep 17 00:00:00 2001 From: manhtukhang Date: Wed, 24 Apr 2024 17:28:29 +0700 Subject: [PATCH 3/4] refactor: DRY tests Signed-off-by: manhtukhang --- src/path_config_test.go | 81 ++++++++++++++++------------------------- src/test_utils.go | 31 ++++++++++++++++ 2 files changed, 62 insertions(+), 50 deletions(-) create mode 100644 src/test_utils.go diff --git a/src/path_config_test.go b/src/path_config_test.go index edf8e71..1706230 100644 --- a/src/path_config_test.go +++ b/src/path_config_test.go @@ -1,7 +1,6 @@ package nxr import ( - "context" "fmt" "testing" @@ -10,11 +9,11 @@ import ( ) const ( - username = "admin" - password = "Testing!123" - url = "http://localhost:1234" - insecure = true - timeout = 30 + testConfigAdminUsername = "admin" + testConfigAdminPassword = "Testing!123" + testConfigAdminURL = "http://localhost:1234" + testConfigAdminInsecure = true + testConfigAdminTimeout = 30 ) // TestConfig mocks the creation, read, update, and delete @@ -33,22 +32,22 @@ func TestConfig(t *testing.T) { // Missing "username" err = testConfigCreate(b, reqStorage, map[string]interface{}{ - "password": password, - "url": url, + "password": testConfigAdminPassword, + "url": testConfigAdminURL, }) assert.Error(t, err) // Missing "password" err = testConfigCreate(b, reqStorage, map[string]interface{}{ - "username": username, - "url": url, + "username": testConfigAdminUsername, + "url": testConfigAdminURL, }) assert.Error(t, err) // Missing "url" err = testConfigCreate(b, reqStorage, map[string]interface{}{ - "username": username, - "password": password, + "username": testConfigAdminUsername, + "password": testConfigAdminPassword, }) assert.Error(t, err) }) @@ -56,37 +55,37 @@ func TestConfig(t *testing.T) { t.Run("Happy cases", func(t *testing.T) { // No "insecure" + "timeout" err := testConfigCreate(b, reqStorage, map[string]interface{}{ - "username": username, - "password": password, - "url": url, + "username": testConfigAdminUsername, + "password": testConfigAdminPassword, + "url": testConfigAdminURL, }) assert.NoError(t, err) // "insecure" err = testConfigCreate(b, reqStorage, map[string]interface{}{ - "username": username, - "password": password, - "url": url, - "insecure": insecure, + "username": testConfigAdminUsername, + "password": testConfigAdminPassword, + "url": testConfigAdminURL, + "insecure": testConfigAdminInsecure, }) assert.NoError(t, err) // Update after create err = testConfigUpdate(b, reqStorage, map[string]interface{}{ - "username": username, - "password": password, - "url": url, - "insecure": !insecure, - "timeout": timeout, + "username": testConfigAdminUsername, + "password": testConfigAdminPassword, + "url": testConfigAdminURL, + "insecure": !testConfigAdminInsecure, + "timeout": testConfigAdminTimeout, }) assert.NoError(t, err) // Read after update err = testConfigRead(b, reqStorage, map[string]interface{}{ - "username": username, - "url": url, - "insecure": !insecure, - "timeout": timeout, + "username": testConfigAdminUsername, + "url": testConfigAdminURL, + "insecure": !testConfigAdminInsecure, + "timeout": testConfigAdminTimeout, }) assert.NoError(t, err) @@ -97,11 +96,7 @@ func TestConfig(t *testing.T) { } func testConfigDelete(b logical.Backend, s logical.Storage) error { - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.DeleteOperation, - Path: configAdminPath, - Storage: s, - }) + resp, err := deleteConfigAdmin(b, s) if err != nil { return err } @@ -113,12 +108,7 @@ func testConfigDelete(b logical.Backend, s logical.Storage) error { } func testConfigCreate(b logical.Backend, s logical.Storage, d map[string]interface{}) error { - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.CreateOperation, - Path: configAdminPath, - Data: d, - Storage: s, - }) + resp, err := writeConfigAdmin(b, s, d) if err != nil { return err } @@ -130,12 +120,7 @@ func testConfigCreate(b logical.Backend, s logical.Storage, d map[string]interfa } func testConfigUpdate(b logical.Backend, s logical.Storage, d map[string]interface{}) error { - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: configAdminPath, - Data: d, - Storage: s, - }) + resp, err := writeConfigAdmin(b, s, d) if err != nil { return err } @@ -147,11 +132,7 @@ func testConfigUpdate(b logical.Backend, s logical.Storage, d map[string]interfa } func testConfigRead(b logical.Backend, s logical.Storage, expected map[string]interface{}) error { - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ReadOperation, - Path: configAdminPath, - Storage: s, - }) + resp, err := readConfigAdmin(b, s) if err != nil { return err } diff --git a/src/test_utils.go b/src/test_utils.go new file mode 100644 index 0000000..24f98d8 --- /dev/null +++ b/src/test_utils.go @@ -0,0 +1,31 @@ +package nxr + +import ( + "context" + "github.com/hashicorp/vault/sdk/logical" +) + +func writeConfigAdmin(b logical.Backend, s logical.Storage, d map[string]interface{}) (*logical.Response, error) { + return b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: configAdminPath, + Storage: s, + Data: d, + }) +} + +func readConfigAdmin(b logical.Backend, s logical.Storage) (*logical.Response, error) { + return b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: configAdminPath, + Storage: s, + }) +} + +func deleteConfigAdmin(b logical.Backend, s logical.Storage) (*logical.Response, error) { + return b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.DeleteOperation, + Path: configAdminPath, + Storage: s, + }) +} From e1f843ef3d628bc26a7729669216a507baecad8e Mon Sep 17 00:00:00 2001 From: manhtukhang Date: Wed, 24 Apr 2024 17:45:12 +0700 Subject: [PATCH 4/4] update(ci): removed unnecessary steps in test workflow Signed-off-by: manhtukhang --- .github/workflows/test.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index eafe0c0..8b5ce9e 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -31,8 +31,6 @@ jobs: uses: actions/setup-go@v5 with: go-version: ${{ matrix.go }} - - name: Setup tools - run: sudo apt-get update && sudo apt-get install -y make - name: Setup gotest run: curl -L https://gotest-release.s3.amazonaws.com/gotest_linux > gotest && chmod +x gotest && sudo mv gotest /usr/bin/gotest #