Added support for digest auth and ACLs

.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:

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:
 

README.md

@@ -37,8 +37,8 @@ workflow.
 ## 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

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))
 
+<a id="nestedatt--acl"></a>
+### Nested Schema for `acl`
+
+Read-Only:
+
+- `id` (String)
+- `permissions` (Number)
+- `scheme` (String)
+
+
 <a id="nestedatt--stat"></a>
 ### Nested Schema for `stat`
 

docs/index.md

@@ -60,8 +60,10 @@ provider "zookeeper" {
 
 ### Optional
 
+- `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. Can be set via `ZOOKEEPER_USERNAME` environment variable.
 
 ## Important aspects about ZooKeeper and this provider
 

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))
 
+<a id="nestedblock--acl"></a>
+### 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'.
+
+
 <a id="nestedatt--stat"></a>
 ### Nested Schema for `stat`
 

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))
 
+<a id="nestedblock--acl"></a>
+### 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'.
+
+
 <a id="nestedatt--stat"></a>
 ### Nested Schema for `stat`
 

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.
@@ -64,17 +65,34 @@ 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.
-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)
 	if err != nil {
 		return nil, fmt.Errorf("unable to connect to ZooKeeper: %w", err)
 	}
 
+	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))
+		if err != nil {
+			return nil, fmt.Errorf("unable to add digest auth: %w", err)
+		}
+	}
+
 	return &Client{
 		zkConn: conn,
 	}, nil
@@ -99,16 +117,16 @@ 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.
 //
 // 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)
 	}
@@ -130,10 +148,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)
 }
 
@@ -202,17 +217,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
@@ -222,6 +243,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)
@@ -230,6 +256,11 @@ func (c *Client) Update(path string, data []byte) (*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.

internal/client/client_test.go

@@ -3,6 +3,7 @@ package client_test
 import (
 	"testing"
 
+	"github.com/go-zookeeper/zk"
 	testifyAssert "github.com/stretchr/testify/assert"
 	"github.com/tfzk/terraform-provider-zookeeper/internal/client"
 )
@@ -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)
 
@@ -82,22 +83,87 @@ 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)
 
-	_, 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())
 }
 
 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 +176,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())
 }