From 5fcaac4591413ec07b56a89cffb2a830bd82492a Mon Sep 17 00:00:00 2001 From: Andrew Barabash Date: Tue, 3 Sep 2024 11:31:30 +0200 Subject: [PATCH 1/7] Implemented digest auth --- internal/client/client.go | 13 +++++++++++-- internal/provider/data_source_znode_test.go | 3 ++- internal/provider/provider.go | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index 88699ed..e0721a1 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -67,7 +67,7 @@ const ( ) // NewClient constructs a new Client instance. -func NewClient(servers string, sessionTimeoutSec int) (*Client, error) { +func NewClient(servers string, sessionTimeoutSec int, username string, password string) (*Client, error) { serversSplit := strings.Split(servers, serversStringSeparator) conn, _, err := zk.Connect(zk.FormatServers(serversSplit), time.Duration(sessionTimeoutSec)*time.Second) @@ -75,6 +75,15 @@ func NewClient(servers string, sessionTimeoutSec int) (*Client, error) { return nil, fmt.Errorf("unable to connect to ZooKeeper: %w", err) } + if username != "" && password != "" { + auth := "digest" + credentials := fmt.Sprintf("%s:%s", username, password) + err = conn.AddAuth(auth, []byte(credentials)) + if err != nil { + return nil, fmt.Errorf("unable to add digest auth: %w", err) + } + } + return &Client{ zkConn: conn, }, nil @@ -99,7 +108,7 @@ func NewClientFromEnv() (*Client, error) { return nil, fmt.Errorf("failed to convert '%s' to integer: %w", zkSession, err) } - return NewClient(zkServers, zkSessionInt) + return NewClient(zkServers, zkSessionInt, "", "") } // Create a ZNode at the given path. diff --git a/internal/provider/data_source_znode_test.go b/internal/provider/data_source_znode_test.go index d073189..4c861c7 100644 --- a/internal/provider/data_source_znode_test.go +++ b/internal/provider/data_source_znode_test.go @@ -23,7 +23,8 @@ func TestAccDataSourceZNode(t *testing.T) { data = "Forza Napoli!" } data "zookeeper_znode" "dst" { - path = zookeeper_znode.src.path + depends_on = [zookeeper_znode.src] + path = zookeeper_znode.src.path }`, srcPath, ), Check: resource.ComposeAggregateTestCheckFunc( diff --git a/internal/provider/provider.go b/internal/provider/provider.go index d58858d..48d2a7f 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -26,6 +26,18 @@ func New() (*schema.Provider, error) { Description: "How many seconds a session is considered valid after losing connectivity. " + "More information about ZooKeeper sessions can be found [here](#zookeeper-sessions).", }, + "username": { + Type: schema.TypeString, + Optional: true, + Sensitive: true, + Description: "Username for digest authentication", + }, + "password": { + Type: schema.TypeString, + Optional: true, + Sensitive: true, + Description: "Password for digest authentication", + }, }, ResourcesMap: map[string]*schema.Resource{ "zookeeper_znode": resourceZNode(), @@ -41,9 +53,11 @@ func New() (*schema.Provider, error) { func configureProviderContext(_ context.Context, rscData *schema.ResourceData) (interface{}, diag.Diagnostics) { servers := rscData.Get("servers").(string) sessionTimeout := rscData.Get("session_timeout").(int) + username := rscData.Get("username").(string) + password := rscData.Get("password").(string) if servers != "" { - c, err := client.NewClient(servers, sessionTimeout) + c, err := client.NewClient(servers, sessionTimeout, username, password) if err != nil { // Report inability to connect internal Client From ee679fca5a90b07f26beddefee48b6cd36b636c9 Mon Sep 17 00:00:00 2001 From: Andrew Barabash Date: Wed, 4 Sep 2024 02:10:31 +0200 Subject: [PATCH 2/7] Implemented ACLs --- internal/client/client.go | 24 +++--- internal/client/client_test.go | 19 ++--- internal/provider/common.go | 40 ++++++++++ internal/provider/data_source_znode.go | 28 +++++++ internal/provider/data_source_znode_test.go | 7 ++ .../provider/resource_sequential_znode.go | 35 ++++++++- .../resource_sequential_znode_test.go | 75 +++++++++++++++++++ internal/provider/resource_znode.go | 45 ++++++++++- internal/provider/resource_znode_test.go | 59 +++++++++++++++ 9 files changed, 309 insertions(+), 23 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index e0721a1..de67ffe 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -30,6 +30,7 @@ type ZNode struct { Path string Stat *zk.Stat Data []byte + ACL []zk.ACL } // Re-exporting errors from ZK library for better encapsulation. @@ -114,10 +115,7 @@ func NewClientFromEnv() (*Client, error) { // Create a ZNode at the given path. // // Note that any necessary ZNode parents will be created if absent. -func (c *Client) Create(path string, data []byte) (*ZNode, error) { - // TODO Make ACL configurable - acl := zk.WorldACL(zk.PermRead | zk.PermWrite | zk.PermCreate | zk.PermDelete) - +func (c *Client) Create(path string, data []byte, acl []zk.ACL) (*ZNode, error) { if path[len(path)-1] == zNodePathSeparator { return nil, fmt.Errorf("non-sequential ZNode cannot have path '%s' because it ends in '%c'", path, zNodePathSeparator) } @@ -139,10 +137,7 @@ func (c *Client) Create(path string, data []byte) (*ZNode, error) { // - created znode path -> `/this/is/a/path/0000000001` // // Note also that any necessary ZNode parents will be created if absent. -func (c *Client) CreateSequential(path string, data []byte) (*ZNode, error) { - // TODO Make ACL configurable - acl := zk.WorldACL(zk.PermRead | zk.PermWrite | zk.PermCreate | zk.PermDelete) - +func (c *Client) CreateSequential(path string, data []byte, acl []zk.ACL) (*ZNode, error) { return c.doCreate(path, data, zk.FlagSequence, acl) } @@ -211,17 +206,23 @@ func (c *Client) Read(path string) (*ZNode, error) { return nil, fmt.Errorf("failed to read ZNode '%s': %w", path, err) } + acls, _, err := c.zkConn.GetACL(path) + if err != nil { + return nil, fmt.Errorf("failed to fetch ACLs for ZNode '%s': %w", path, err) + } + return &ZNode{ Path: path, Stat: stat, Data: data, + ACL: acls, }, nil } // Update the ZNode at the given path, under the assumption that it is there. // // Will return an error if it doesn't already exist. -func (c *Client) Update(path string, data []byte) (*ZNode, error) { +func (c *Client) Update(path string, data []byte, acl []zk.ACL) (*ZNode, error) { exists, err := c.Exists(path) if err != nil { return nil, err @@ -231,6 +232,11 @@ func (c *Client) Update(path string, data []byte) (*ZNode, error) { return nil, fmt.Errorf("failed to update ZNode '%s': does not exist", path) } + _, err = c.zkConn.SetACL(path, acl, matchAnyVersion) + if err != nil { + return nil, fmt.Errorf("failed to update ZNode '%s' ACL: %w", path, err) + } + _, err = c.zkConn.Set(path, data, matchAnyVersion) if err != nil { return nil, fmt.Errorf("failed to update ZNode '%s': %w", path, err) diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 49031ec..5fb622d 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -1,6 +1,7 @@ package client_test import ( + "github.com/go-zookeeper/zk" "testing" testifyAssert "github.com/stretchr/testify/assert" @@ -25,7 +26,7 @@ func TestClassicCRUD(t *testing.T) { assert.False(znodeExists) // create - znode, err := client.Create("/test/ClassicCRUD", []byte("one")) + znode, err := client.Create("/test/ClassicCRUD", []byte("one"), zk.WorldACL(zk.PermAll)) assert.NoError(err) assert.Equal("/test/ClassicCRUD", znode.Path) assert.Equal([]byte("one"), znode.Data) @@ -42,7 +43,7 @@ func TestClassicCRUD(t *testing.T) { assert.Equal([]byte("one"), znode.Data) // update - znode, err = client.Update("/test/ClassicCRUD", []byte("two")) + znode, err = client.Update("/test/ClassicCRUD", []byte("two"), zk.WorldACL(zk.PermAll)) assert.NoError(err) assert.Equal("/test/ClassicCRUD", znode.Path) assert.Equal([]byte("two"), znode.Data) @@ -69,11 +70,11 @@ func TestClassicCRUD(t *testing.T) { func TestCreateSequential(t *testing.T) { client, assert := initTest(t) - noPrefixSeqZNode, err := client.CreateSequential("/test/CreateSequential/", []byte("seq")) + noPrefixSeqZNode, err := client.CreateSequential("/test/CreateSequential/", []byte("seq"), zk.WorldACL(zk.PermAll)) assert.NoError(err) assert.Equal("/test/CreateSequential/0000000000", noPrefixSeqZNode.Path) - prefixSeqZNode, err := client.CreateSequential("/test/CreateSequentialWithPrefix/prefix-", []byte("seq")) + prefixSeqZNode, err := client.CreateSequential("/test/CreateSequentialWithPrefix/prefix-", []byte("seq"), zk.WorldACL(zk.PermAll)) assert.NoError(err) assert.Equal("/test/CreateSequentialWithPrefix/prefix-0000000000", prefixSeqZNode.Path) @@ -85,7 +86,7 @@ func TestCreateSequential(t *testing.T) { func TestFailureWhenCreatingForNonSequentialZNodeEndingInSlash(t *testing.T) { client, assert := initTest(t) - _, err := client.Create("/test/willFail/", nil) + _, err := client.Create("/test/willFail/", nil, zk.WorldACL(zk.PermAll)) assert.Error(err) assert.Equal("non-sequential ZNode cannot have path '/test/willFail/' because it ends in '/'", err.Error()) } @@ -93,11 +94,11 @@ func TestFailureWhenCreatingForNonSequentialZNodeEndingInSlash(t *testing.T) { func TestFailureWhenCreatingWhenZNodeAlreadyExists(t *testing.T) { client, assert := initTest(t) - _, err := client.Create("/test/node", nil) + _, err := client.Create("/test/node", nil, zk.WorldACL(zk.PermAll)) assert.NoError(err) - _, err = client.Create("/test/node", nil) + _, err = client.Create("/test/node", nil, zk.WorldACL(zk.PermAll)) assert.Error(err) - assert.Equal("failed to create ZNode '/test/node' (size: 0, createFlags: 0, acl: [{15 world anyone}]): zk: node already exists", err.Error()) + assert.Equal("failed to create ZNode '/test/node' (size: 0, createFlags: 0, acl: [{31 world anyone}]): zk: node already exists", err.Error()) err = client.Delete("/test") assert.NoError(err) @@ -110,7 +111,7 @@ func TestFailureWithNonExistingZNodes(t *testing.T) { assert.Error(err) assert.Equal("failed to read ZNode '/does-not-exist': zk: node does not exist", err.Error()) - _, err = client.Update("/also-does-not-exist", nil) + _, err = client.Update("/also-does-not-exist", nil, zk.WorldACL(zk.PermAll)) assert.Error(err) assert.Equal("failed to update ZNode '/also-does-not-exist': does not exist", err.Error()) } diff --git a/internal/provider/common.go b/internal/provider/common.go index 4c2b186..5012d4e 100644 --- a/internal/provider/common.go +++ b/internal/provider/common.go @@ -3,6 +3,7 @@ package provider import ( "encoding/base64" "fmt" + "github.com/go-zookeeper/zk" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -31,6 +32,21 @@ func setAttributesFromZNode(rscData *schema.ResourceData, znode *client.ZNode, d diags = append(diags, diag.FromErr(err)...) } + // Convert ACLs from []zk.ACL to []map[string]interface{} + var aclConfigs []map[string]interface{} + for _, acl := range znode.ACL { + aclConfig := map[string]interface{}{ + "scheme": acl.Scheme, + "id": acl.ID, + "permissions": acl.Perms, + } + aclConfigs = append(aclConfigs, aclConfig) + } + + if err := rscData.Set("acl", aclConfigs); err != nil { + diags = append(diags, diag.FromErr(err)...) + } + return diags } @@ -141,3 +157,27 @@ func getDataBytesFromResourceData(rscData *schema.ResourceData) ([]byte, error) return nil, nil } + +func parseACLsFromResourceData(rscData *schema.ResourceData) ([]zk.ACL, error) { + aclConfigs := rscData.Get("acl").([]interface{}) + var acls []zk.ACL + + for _, aclConfig := range aclConfigs { + aclMap := aclConfig.(map[string]interface{}) + scheme := aclMap["scheme"].(string) + id := aclMap["id"].(string) + permissions := aclMap["permissions"].(int) + + acls = append(acls, zk.ACL{ + Scheme: scheme, + ID: id, + Perms: int32(permissions), + }) + } + + if len(acls) == 0 { + acls = zk.WorldACL(zk.PermAll) + } + + return acls, nil +} diff --git a/internal/provider/data_source_znode.go b/internal/provider/data_source_znode.go index e371914..fa0d8fe 100644 --- a/internal/provider/data_source_znode.go +++ b/internal/provider/data_source_znode.go @@ -29,6 +29,34 @@ func datasourceZNode() *schema.Resource { "Use this if content is binary (i.e. sequence of bytes).", }, "stat": statSchema(), + "acl": { + Type: schema.TypeList, + Optional: true, + Computed: true, + Description: "List of ACL entries for the ZNode.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "scheme": { + Type: schema.TypeString, + Required: true, + Description: "The ACL scheme, such as 'world', 'digest', " + + "'ip', 'x509'.", + }, + "id": { + Type: schema.TypeString, + Required: true, + Description: "The ID for the ACL entry. For example, " + + "user:hash in 'digest' scheme.", + }, + "permissions": { + Type: schema.TypeInt, + Required: true, + Description: "The permissions for the ACL entry, " + + "represented as an integer bitmask.", + }, + }, + }, + }, }, Description: "Provides access to the content of a " + zNodeLinkForDesc + ". " + diff --git a/internal/provider/data_source_znode_test.go b/internal/provider/data_source_znode_test.go index 4c861c7..244da44 100644 --- a/internal/provider/data_source_znode_test.go +++ b/internal/provider/data_source_znode_test.go @@ -59,6 +59,13 @@ func TestAccDataSourceZNode(t *testing.T) { resource.TestCheckResourceAttrPair("data.zookeeper_znode.dst", "stat.0.num_children", "zookeeper_znode.src", "stat.0.num_children"), resource.TestCheckResourceAttr("data.zookeeper_znode.dst", "stat.0.num_children", "0"), + + resource.TestCheckResourceAttrPair("data.zookeeper_znode.dst", "acl.0.scheme", "zookeeper_znode.src", "acl.0.scheme"), + resource.TestCheckResourceAttr("data.zookeeper_znode.dst", "acl.0.scheme", "world"), + resource.TestCheckResourceAttrPair("data.zookeeper_znode.dst", "acl.0.id", "zookeeper_znode.src", "acl.0.id"), + resource.TestCheckResourceAttr("data.zookeeper_znode.dst", "acl.0.id", "anyone"), + resource.TestCheckResourceAttrPair("data.zookeeper_znode.dst", "acl.0.permissions", "zookeeper_znode.src", "acl.0.permissions"), + resource.TestCheckResourceAttr("data.zookeeper_znode.dst", "acl.0.permissions", "31"), ), }, }, diff --git a/internal/provider/resource_sequential_znode.go b/internal/provider/resource_sequential_znode.go index 9298512..70578ad 100644 --- a/internal/provider/resource_sequential_znode.go +++ b/internal/provider/resource_sequential_znode.go @@ -53,6 +53,34 @@ func resourceSeqZNode() *schema.Resource { "The prefix of this will match `path_prefix`.", }, "stat": statSchema(), + "acl": { + Type: schema.TypeList, + Optional: true, + Computed: true, + Description: "List of ACL entries for the ZNode.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "scheme": { + Type: schema.TypeString, + Required: true, + Description: "The ACL scheme, such as 'world', 'digest', " + + "'ip', 'x509'.", + }, + "id": { + Type: schema.TypeString, + Required: true, + Description: "The ID for the ACL entry. For example, " + + "user:hash in 'digest' scheme.", + }, + "permissions": { + Type: schema.TypeInt, + Required: true, + Description: "The permissions for the ACL entry, " + + "represented as an integer bitmask.", + }, + }, + }, + }, }, Description: "Manages the lifecycle of a " + zNodeLinkForDesc + ". " + @@ -72,7 +100,12 @@ func resourceSeqZNodeCreate(_ context.Context, rscData *schema.ResourceData, prv return diag.FromErr(err) } - znode, err := zkClient.CreateSequential(znodePathPrefix, dataBytes) + acls, err := parseACLsFromResourceData(rscData) + if err != nil { + return diag.FromErr(err) + } + + znode, err := zkClient.CreateSequential(znodePathPrefix, dataBytes, acls) if err != nil { return diag.Errorf("Failed to create Sequential ZNode '%s': %v", znodePathPrefix, err) } diff --git a/internal/provider/resource_sequential_znode_test.go b/internal/provider/resource_sequential_znode_test.go index f73d561..d0451d3 100644 --- a/internal/provider/resource_sequential_znode_test.go +++ b/internal/provider/resource_sequential_znode_test.go @@ -70,3 +70,78 @@ func TestAccResourceSeqZNode_FromPrefix(t *testing.T) { }, }) } + +func TestAccResourceSeqZNode_DefaultACL(t *testing.T) { + seqFromDir := "/" + acctest.RandString(10) + "/" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { checkPreconditions(t) }, + ProviderFactories: providerFactoriesMap(), + CheckDestroy: confirmAllZNodeDestroyed, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(` + resource "zookeeper_sequential_znode" "default_acl" { + path_prefix = "%s" + data = "sequential znode created with default acl" + }`, seqFromDir, + ), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestMatchResourceAttr("zookeeper_sequential_znode.default_acl", "path", regexp.MustCompile(`^`+seqFromDir+`\d{10}`)), + resource.TestCheckResourceAttrPair("zookeeper_sequential_znode.default_acl", "path", "zookeeper_sequential_znode.default_acl", "id"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.default_acl", "data", "sequential znode created with default acl"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.default_acl", "data_base64", "c2VxdWVudGlhbCB6bm9kZSBjcmVhdGVkIHdpdGggZGVmYXVsdCBhY2w="), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.default_acl", "acl.#", "1"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.default_acl", "acl.0.scheme", "world"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.default_acl", "acl.0.id", "anyone"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.default_acl", "acl.0.permissions", "31"), + ), + }, + { + ResourceName: "zookeeper_sequential_znode.default_acl", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccResourceSeqZNode_WithACL(t *testing.T) { + seqFromDir := "/" + acctest.RandString(10) + "/" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { checkPreconditions(t) }, + ProviderFactories: providerFactoriesMap(), + CheckDestroy: confirmAllZNodeDestroyed, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(` + resource "zookeeper_sequential_znode" "with_acl" { + path_prefix = "%s" + data = "sequential znode created with acl" + acl { + scheme = "world" + id = "anyone" + permissions = 31 + } + }`, seqFromDir, + ), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestMatchResourceAttr("zookeeper_sequential_znode.with_acl", "path", regexp.MustCompile(`^`+seqFromDir+`\d{10}`)), + resource.TestCheckResourceAttrPair("zookeeper_sequential_znode.with_acl", "path", "zookeeper_sequential_znode.with_acl", "id"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.with_acl", "data", "sequential znode created with acl"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.with_acl", "data_base64", "c2VxdWVudGlhbCB6bm9kZSBjcmVhdGVkIHdpdGggYWNs"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.with_acl", "acl.#", "1"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.with_acl", "acl.0.scheme", "world"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.with_acl", "acl.0.id", "anyone"), + resource.TestCheckResourceAttr("zookeeper_sequential_znode.with_acl", "acl.0.permissions", "31"), + ), + }, + { + ResourceName: "zookeeper_sequential_znode.with_acl", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} diff --git a/internal/provider/resource_znode.go b/internal/provider/resource_znode.go index 76af5b6..8ff2fcd 100644 --- a/internal/provider/resource_znode.go +++ b/internal/provider/resource_znode.go @@ -3,7 +3,6 @@ package provider import ( "context" "errors" - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/tfzk/terraform-provider-zookeeper/internal/client" @@ -42,6 +41,34 @@ func resourceZNode() *schema.Resource { "Mutually exclusive with `data`.", }, "stat": statSchema(), + "acl": { + Type: schema.TypeList, + Optional: true, + Computed: true, + Description: "List of ACL entries for the ZNode.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "scheme": { + Type: schema.TypeString, + Required: true, + Description: "The ACL scheme, such as 'world', 'digest', " + + "'ip', 'x509'.", + }, + "id": { + Type: schema.TypeString, + Required: true, + Description: "The ID for the ACL entry. For example, " + + "user:hash in 'digest' scheme.", + }, + "permissions": { + Type: schema.TypeInt, + Required: true, + Description: "The permissions for the ACL entry, " + + "represented as an integer bitmask.", + }, + }, + }, + }, }, Description: "Manages the lifecycle of a " + zNodeLinkForDesc + ". " + @@ -61,7 +88,12 @@ func resourceZNodeCreate(_ context.Context, rscData *schema.ResourceData, prvCli return diag.FromErr(err) } - znode, err := zkClient.Create(znodePath, dataBytes) + acls, err := parseACLsFromResourceData(rscData) + if err != nil { + return diag.FromErr(err) + } + + znode, err := zkClient.Create(znodePath, dataBytes, acls) if err != nil { return diag.Errorf("Failed to create ZNode '%s': %v", znodePath, err) } @@ -98,13 +130,18 @@ func resourceZNodeUpdate(_ context.Context, rscData *schema.ResourceData, prvCli znodePath := rscData.Id() - if rscData.HasChanges("data", "data_base64") { + if rscData.HasChanges("data", "data_base64", "acl") { dataBytes, err := getDataBytesFromResourceData(rscData) if err != nil { return diag.FromErr(err) } - znode, err := zkClient.Update(znodePath, dataBytes) + acls, err := parseACLsFromResourceData(rscData) + if err != nil { + return diag.FromErr(err) + } + + znode, err := zkClient.Update(znodePath, dataBytes, acls) if err != nil { return diag.Errorf("Failed to update ZNode '%s': %v", znodePath, err) } diff --git a/internal/provider/resource_znode_test.go b/internal/provider/resource_znode_test.go index f8ff177..8ad8f5e 100644 --- a/internal/provider/resource_znode_test.go +++ b/internal/provider/resource_znode_test.go @@ -132,3 +132,62 @@ func TestAccResourceZNode_Base64(t *testing.T) { }, }) } + +func TestAccResourceZNode_DefaultACL(t *testing.T) { + path := "/" + acctest.RandString(10) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { checkPreconditions(t) }, + ProviderFactories: providerFactoriesMap(), + CheckDestroy: confirmAllZNodeDestroyed, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(` + resource "zookeeper_znode" "test_default_acl" { + path = "%s" + data = "Default ACL Test" + }`, path), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("zookeeper_znode.test_default_acl", "path", path), + resource.TestCheckResourceAttr("zookeeper_znode.test_default_acl", "data", "Default ACL Test"), + resource.TestCheckResourceAttr("zookeeper_znode.test_default_acl", "acl.#", "1"), + resource.TestCheckResourceAttr("zookeeper_znode.test_default_acl", "acl.0.scheme", "world"), + resource.TestCheckResourceAttr("zookeeper_znode.test_default_acl", "acl.0.id", "anyone"), + resource.TestCheckResourceAttr("zookeeper_znode.test_default_acl", "acl.0.permissions", "31"), + ), + }, + }, + }) +} + +func TestAccResourceZNode_WithACL(t *testing.T) { + path := "/" + acctest.RandString(10) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { checkPreconditions(t) }, + ProviderFactories: providerFactoriesMap(), + CheckDestroy: confirmAllZNodeDestroyed, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(` + resource "zookeeper_znode" "test_acl" { + path = "%s" + data = "ACL Test" + acl { + scheme = "world" + id = "anyone" + permissions = 31 + } + }`, path), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("zookeeper_znode.test_acl", "path", path), + resource.TestCheckResourceAttr("zookeeper_znode.test_acl", "data", "ACL Test"), + resource.TestCheckResourceAttr("zookeeper_znode.test_acl", "acl.#", "1"), + resource.TestCheckResourceAttr("zookeeper_znode.test_acl", "acl.0.scheme", "world"), + resource.TestCheckResourceAttr("zookeeper_znode.test_acl", "acl.0.id", "anyone"), + resource.TestCheckResourceAttr("zookeeper_znode.test_acl", "acl.0.permissions", "31"), + ), + }, + }, + }) +} From ad5d9c99ec5ad293fc6e75e83e18a37e95a534e5 Mon Sep 17 00:00:00 2001 From: Andrew Barabash Date: Wed, 4 Sep 2024 02:16:24 +0200 Subject: [PATCH 3/7] Generated docs --- docs/data-sources/znode.md | 11 +++++++++++ docs/index.md | 2 ++ docs/resources/sequential_znode.md | 11 +++++++++++ docs/resources/znode.md | 11 +++++++++++ internal/provider/data_source_znode.go | 1 - 5 files changed, 35 insertions(+), 1 deletion(-) diff --git a/docs/data-sources/znode.md b/docs/data-sources/znode.md index 445e240..ee1d17a 100644 --- a/docs/data-sources/znode.md +++ b/docs/data-sources/znode.md @@ -44,11 +44,22 @@ output "best_team_znode_data_base64" { ### Read-Only +- `acl` (List of Object) List of ACL entries for the ZNode. (see [below for nested schema](#nestedatt--acl)) - `data` (String) Content of the ZNode. Use this if content is a UTF-8 string. - `data_base64` (String) Content of the ZNode, encoded in Base64. Use this if content is binary (i.e. sequence of bytes). - `id` (String) The ID of this resource. - `stat` (List of Object) [ZooKeeper Stat Structure](https://zookeeper.apache.org/doc/current/zookeeperProgrammers.html#sc_zkStatStructure) of the ZNode. More details about `stat` can be found [here](../../docs#the-stat-structure). (see [below for nested schema](#nestedatt--stat)) + +### Nested Schema for `acl` + +Read-Only: + +- `id` (String) +- `permissions` (Number) +- `scheme` (String) + + ### Nested Schema for `stat` diff --git a/docs/index.md b/docs/index.md index 82720ed..d55a52c 100644 --- a/docs/index.md +++ b/docs/index.md @@ -60,8 +60,10 @@ provider "zookeeper" { ### Optional +- `password` (String, Sensitive) Password for digest authentication - `servers` (String) A comma separated list of 'host:port' pairs, pointing at ZooKeeper Server(s). - `session_timeout` (Number) How many seconds a session is considered valid after losing connectivity. More information about ZooKeeper sessions can be found [here](#zookeeper-sessions). +- `username` (String, Sensitive) Username for digest authentication ## Important aspects about ZooKeeper and this provider diff --git a/docs/resources/sequential_znode.md b/docs/resources/sequential_znode.md index e9e0fa7..e827dbc 100644 --- a/docs/resources/sequential_znode.md +++ b/docs/resources/sequential_znode.md @@ -62,6 +62,7 @@ resource "zookeeper_sequential_znode" "seqC" { ### Optional +- `acl` (Block List) List of ACL entries for the ZNode. (see [below for nested schema](#nestedblock--acl)) - `data` (String) Content to store in the ZNode, as a UTF-8 string. Mutually exclusive with `data_base64`. - `data_base64` (String) Content to store in the ZNode, as Base64 encoded bytes. Mutually exclusive with `data`. @@ -71,6 +72,16 @@ resource "zookeeper_sequential_znode" "seqC" { - `path` (String) Absolute path to the Sequential ZNode, once it is created. The prefix of this will match `path_prefix`. - `stat` (List of Object) [ZooKeeper Stat Structure](https://zookeeper.apache.org/doc/current/zookeeperProgrammers.html#sc_zkStatStructure) of the ZNode. More details about `stat` can be found [here](../../docs#the-stat-structure). (see [below for nested schema](#nestedatt--stat)) + +### Nested Schema for `acl` + +Required: + +- `id` (String) The ID for the ACL entry. For example, user:hash in 'digest' scheme. +- `permissions` (Number) The permissions for the ACL entry, represented as an integer bitmask. +- `scheme` (String) The ACL scheme, such as 'world', 'digest', 'ip', 'x509'. + + ### Nested Schema for `stat` diff --git a/docs/resources/znode.md b/docs/resources/znode.md index cf136b8..dad6a13 100644 --- a/docs/resources/znode.md +++ b/docs/resources/znode.md @@ -35,6 +35,7 @@ resource "zookeeper_znode" "napoli_logo" { ### Optional +- `acl` (Block List) List of ACL entries for the ZNode. (see [below for nested schema](#nestedblock--acl)) - `data` (String) Content to store in the ZNode, as a UTF-8 string. Mutually exclusive with `data_base64`. - `data_base64` (String) Content to store in the ZNode, as Base64 encoded bytes. Mutually exclusive with `data`. @@ -43,6 +44,16 @@ resource "zookeeper_znode" "napoli_logo" { - `id` (String) The ID of this resource. - `stat` (List of Object) [ZooKeeper Stat Structure](https://zookeeper.apache.org/doc/current/zookeeperProgrammers.html#sc_zkStatStructure) of the ZNode. More details about `stat` can be found [here](../../docs#the-stat-structure). (see [below for nested schema](#nestedatt--stat)) + +### Nested Schema for `acl` + +Required: + +- `id` (String) The ID for the ACL entry. For example, user:hash in 'digest' scheme. +- `permissions` (Number) The permissions for the ACL entry, represented as an integer bitmask. +- `scheme` (String) The ACL scheme, such as 'world', 'digest', 'ip', 'x509'. + + ### Nested Schema for `stat` diff --git a/internal/provider/data_source_znode.go b/internal/provider/data_source_znode.go index fa0d8fe..b9cd68b 100644 --- a/internal/provider/data_source_znode.go +++ b/internal/provider/data_source_znode.go @@ -31,7 +31,6 @@ func datasourceZNode() *schema.Resource { "stat": statSchema(), "acl": { Type: schema.TypeList, - Optional: true, Computed: true, Description: "List of ACL entries for the ZNode.", Elem: &schema.Resource{ From 906bbbacbff02102e8d83815337a482b47f6e860 Mon Sep 17 00:00:00 2001 From: Andrew Barabash Date: Fri, 20 Sep 2024 14:20:52 +0200 Subject: [PATCH 4/7] Updated readme --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f3db5d1..6af9a7a 100644 --- a/README.md +++ b/README.md @@ -31,8 +31,8 @@ version it implements, and Terraform: ## Provider features * [x] support for ZK standard multi-server connection string -* [ ] support for ZK authentication -* [ ] support for ZK ACLs +* [x] support for ZK authentication +* [x] support for ZK ACLs * [x] "session timeout" configuration * [x] create ZNode * [x] create Sequential ZNode From 3003b98acaf4ee31784e1fd3df0f8f6afeed3bb3 Mon Sep 17 00:00:00 2001 From: Andrew Barabash Date: Tue, 1 Oct 2024 03:23:37 +0200 Subject: [PATCH 5/7] Added credentials via env and new tests --- internal/client/client.go | 15 ++++++- internal/client/client_test.go | 67 ++++++++++++++++++++++++++++- internal/provider/common.go | 18 +++++--- internal/provider/provider.go | 6 ++- internal/provider/resource_znode.go | 1 + 5 files changed, 97 insertions(+), 10 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index de67ffe..3df87c6 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -65,6 +65,10 @@ const ( // DefaultZooKeeperSessionSec is the default amount of seconds configured for the // Client timeout session, in case EnvZooKeeperSessionSec is not set. DefaultZooKeeperSessionSec = 30 + + // Environment variables to provide digest auth credentials. + EnvZooKeeperUsername = "ZOOKEEPER_USERNAME" + EnvZooKeeperPassword = "ZOOKEEPER_PASSWORD" ) // NewClient constructs a new Client instance. @@ -76,7 +80,11 @@ func NewClient(servers string, sessionTimeoutSec int, username string, password return nil, fmt.Errorf("unable to connect to ZooKeeper: %w", err) } - if username != "" && password != "" { + if (username == "") != (password == "") { + return nil, fmt.Errorf("both username and password must be specified together") + } + + if username != "" { auth := "digest" credentials := fmt.Sprintf("%s:%s", username, password) err = conn.AddAuth(auth, []byte(credentials)) @@ -109,7 +117,10 @@ func NewClientFromEnv() (*Client, error) { return nil, fmt.Errorf("failed to convert '%s' to integer: %w", zkSession, err) } - return NewClient(zkServers, zkSessionInt, "", "") + zkUsername, _ := os.LookupEnv(EnvZooKeeperUsername) + zkPassword, _ := os.LookupEnv(EnvZooKeeperPassword) + + return NewClient(zkServers, zkSessionInt, zkUsername, zkPassword) } // Create a ZNode at the given path. diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 5fb622d..6d39d0a 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -1,9 +1,9 @@ package client_test import ( - "github.com/go-zookeeper/zk" "testing" + "github.com/go-zookeeper/zk" testifyAssert "github.com/stretchr/testify/assert" "github.com/tfzk/terraform-provider-zookeeper/internal/client" ) @@ -83,6 +83,71 @@ func TestCreateSequential(t *testing.T) { assert.NoError(err) } +func TestDigestAuthenticationSuccess(t *testing.T) { + t.Setenv(client.EnvZooKeeperUsername, "username") + t.Setenv(client.EnvZooKeeperPassword, "password") + client, assert := initTest(t) + + // Create a ZNode accessible only by the given user + acl := zk.DigestACL(zk.PermAll, "username", "password") + znode, err := client.Create("/auth-test/DigestAuthentication", []byte("data"), acl) + assert.NoError(err) + assert.Equal("/auth-test/DigestAuthentication", znode.Path) + assert.Equal([]byte("data"), znode.Data) + assert.Equal(acl, znode.ACL) + + // Make sure it's accessible + znode, err = client.Read("/auth-test/DigestAuthentication") + assert.NoError(err) + assert.Equal("/auth-test/DigestAuthentication", znode.Path) + assert.Equal([]byte("data"), znode.Data) + assert.Equal(acl, znode.ACL) + + // Cleanup + err = client.Delete("/auth-test/DigestAuthentication") + assert.NoError(err) + err = client.Delete("/auth-test") + assert.NoError(err) +} + +func TestFailureWhenReadingZNodeWithIncorrectAuth(t *testing.T) { + // Create client authenticated as foo user + t.Setenv(client.EnvZooKeeperUsername, "foo") + t.Setenv(client.EnvZooKeeperPassword, "password") + fooClient, assert := initTest(t) + + // Create a ZNode accessible only by foo user + acl := zk.DigestACL(zk.PermAll, "foo", "password") + znode, err := fooClient.Create("/auth-fail-test/AccessibleOnlyByFoo", []byte("data"), acl) + assert.NoError(err) + assert.Equal("/auth-fail-test/AccessibleOnlyByFoo", znode.Path) + assert.Equal([]byte("data"), znode.Data) + assert.Equal(acl, znode.ACL) + + // Make sure it's accessible by foo user + znode, err = fooClient.Read("/auth-fail-test/AccessibleOnlyByFoo") + assert.NoError(err) + assert.Equal("/auth-fail-test/AccessibleOnlyByFoo", znode.Path) + assert.Equal([]byte("data"), znode.Data) + assert.Equal(acl, znode.ACL) + + // Create client authenticated as bar user + t.Setenv(client.EnvZooKeeperUsername, "bar") + t.Setenv(client.EnvZooKeeperPassword, "password") + barClient, err := client.NewClientFromEnv() + assert.NoError(err) + + // The node should be inaccessible by bar user + _, err = barClient.Read("/auth-fail-test/AccessibleOnlyByFoo") + assert.EqualError(err, "failed to read ZNode '/auth-fail-test/AccessibleOnlyByFoo': zk: not authenticated") + + // Cleanup + err = fooClient.Delete("/auth-fail-test/AccessibleOnlyByFoo") + assert.NoError(err) + err = fooClient.Delete("/auth-fail-test") + assert.NoError(err) +} + func TestFailureWhenCreatingForNonSequentialZNodeEndingInSlash(t *testing.T) { client, assert := initTest(t) diff --git a/internal/provider/common.go b/internal/provider/common.go index 5012d4e..9179256 100644 --- a/internal/provider/common.go +++ b/internal/provider/common.go @@ -3,8 +3,9 @@ package provider import ( "encoding/base64" "fmt" - "github.com/go-zookeeper/zk" + "math" + "github.com/go-zookeeper/zk" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/tfzk/terraform-provider-zookeeper/internal/client" @@ -33,7 +34,7 @@ func setAttributesFromZNode(rscData *schema.ResourceData, znode *client.ZNode, d } // Convert ACLs from []zk.ACL to []map[string]interface{} - var aclConfigs []map[string]interface{} + aclConfigs := make([]map[string]interface{}, 0, len(znode.ACL)) for _, acl := range znode.ACL { aclConfig := map[string]interface{}{ "scheme": acl.Scheme, @@ -160,18 +161,25 @@ func getDataBytesFromResourceData(rscData *schema.ResourceData) ([]byte, error) func parseACLsFromResourceData(rscData *schema.ResourceData) ([]zk.ACL, error) { aclConfigs := rscData.Get("acl").([]interface{}) - var acls []zk.ACL + acls := make([]zk.ACL, 0, len(aclConfigs)) for _, aclConfig := range aclConfigs { aclMap := aclConfig.(map[string]interface{}) scheme := aclMap["scheme"].(string) id := aclMap["id"].(string) - permissions := aclMap["permissions"].(int) + permissionsValue, ok := aclMap["permissions"].(int) + if !ok { + return nil, fmt.Errorf("acl permissions value is not an integer") + } + if permissionsValue < math.MinInt32 || permissionsValue > math.MaxInt32 { + return nil, fmt.Errorf("acl permissions value %d is out of int32 range", permissionsValue) + } + permissions := int32(permissionsValue) acls = append(acls, zk.ACL{ Scheme: scheme, ID: id, - Perms: int32(permissions), + Perms: permissions, }) } diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 48d2a7f..3de7552 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -30,13 +30,15 @@ func New() (*schema.Provider, error) { Type: schema.TypeString, Optional: true, Sensitive: true, - Description: "Username for digest authentication", + DefaultFunc: schema.EnvDefaultFunc(client.EnvZooKeeperUsername, nil), + Description: "Username for digest authentication. Can be set via `ZOOKEEPER_USERNAME` environment variable.", }, "password": { Type: schema.TypeString, Optional: true, Sensitive: true, - Description: "Password for digest authentication", + DefaultFunc: schema.EnvDefaultFunc(client.EnvZooKeeperPassword, nil), + Description: "Password for digest authentication. Can be set via `ZOOKEEPER_PASSWORD` environment variable.", }, }, ResourcesMap: map[string]*schema.Resource{ diff --git a/internal/provider/resource_znode.go b/internal/provider/resource_znode.go index 8ff2fcd..4204dd9 100644 --- a/internal/provider/resource_znode.go +++ b/internal/provider/resource_znode.go @@ -3,6 +3,7 @@ package provider import ( "context" "errors" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/tfzk/terraform-provider-zookeeper/internal/client" From 9d53ce3c9bdd4d90d4d30752521fcefa9ac418cc Mon Sep 17 00:00:00 2001 From: Andrew Barabash Date: Wed, 2 Oct 2024 17:36:06 +0200 Subject: [PATCH 6/7] Updated changelog, run make generate --- CHANGELOG.md | 2 ++ docs/index.md | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd0ab2f..a921cfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ IMPROVEMENTS: * Enabling CI testing for versions `1.9` of Terraform +* Added support for digest authentication in provider configuration +* Added support for ZNode ACL management in `zookeeper_znode` and `zookeeper_sequential_znode` resources NOTES: diff --git a/docs/index.md b/docs/index.md index d55a52c..4661b32 100644 --- a/docs/index.md +++ b/docs/index.md @@ -60,10 +60,10 @@ provider "zookeeper" { ### Optional -- `password` (String, Sensitive) Password for digest authentication +- `password` (String, Sensitive) Password for digest authentication. Can be set via `ZOOKEEPER_PASSWORD` environment variable. - `servers` (String) A comma separated list of 'host:port' pairs, pointing at ZooKeeper Server(s). - `session_timeout` (Number) How many seconds a session is considered valid after losing connectivity. More information about ZooKeeper sessions can be found [here](#zookeeper-sessions). -- `username` (String, Sensitive) Username for digest authentication +- `username` (String, Sensitive) Username for digest authentication. Can be set via `ZOOKEEPER_USERNAME` environment variable. ## Important aspects about ZooKeeper and this provider From 374ad394ae978ef68c4751d2aac94bf986b06101 Mon Sep 17 00:00:00 2001 From: Andrew Barabash Date: Wed, 9 Oct 2024 15:20:54 +0200 Subject: [PATCH 7/7] Fixed tests --- .github/workflows/build-test.yml | 2 ++ internal/client/client.go | 5 +++++ internal/provider/data_source_znode.go | 6 +++--- internal/provider/data_source_znode_test.go | 1 - internal/provider/provider_test.go | 1 + 5 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 14cb824..08cad98 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -60,6 +60,8 @@ jobs: image: zookeeper:3.5 ports: - 2181:2181 + env: + ZOO_MAX_CLIENT_CNXNS: 200 strategy: fail-fast: false matrix: diff --git a/internal/client/client.go b/internal/client/client.go index 3df87c6..5c02566 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -256,6 +256,11 @@ func (c *Client) Update(path string, data []byte, acl []zk.ACL) (*ZNode, error) return c.Read(path) } +// Close the connection. +func (c *Client) Close() { + c.zkConn.Close() +} + // Delete the given ZNode. // // Note that will also delete any child ZNode, recursively. diff --git a/internal/provider/data_source_znode.go b/internal/provider/data_source_znode.go index b9cd68b..a4aef8c 100644 --- a/internal/provider/data_source_znode.go +++ b/internal/provider/data_source_znode.go @@ -37,19 +37,19 @@ func datasourceZNode() *schema.Resource { Schema: map[string]*schema.Schema{ "scheme": { Type: schema.TypeString, - Required: true, + Computed: true, Description: "The ACL scheme, such as 'world', 'digest', " + "'ip', 'x509'.", }, "id": { Type: schema.TypeString, - Required: true, + Computed: true, Description: "The ID for the ACL entry. For example, " + "user:hash in 'digest' scheme.", }, "permissions": { Type: schema.TypeInt, - Required: true, + Computed: true, Description: "The permissions for the ACL entry, " + "represented as an integer bitmask.", }, diff --git a/internal/provider/data_source_znode_test.go b/internal/provider/data_source_znode_test.go index 244da44..beb783d 100644 --- a/internal/provider/data_source_znode_test.go +++ b/internal/provider/data_source_znode_test.go @@ -23,7 +23,6 @@ func TestAccDataSourceZNode(t *testing.T) { data = "Forza Napoli!" } data "zookeeper_znode" "dst" { - depends_on = [zookeeper_znode.src] path = zookeeper_znode.src.path }`, srcPath, ), diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index d81b33d..8363d9b 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -62,5 +62,6 @@ func confirmAllZNodeDestroyed(s *terraform.State) error { } } + zkClient.Close() return nil }