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/README.md b/README.md index a7c3af6..4fbc5be 100644 --- a/README.md +++ b/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 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 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"