lidavidm commented on code in PR #1207:
URL: https://github.com/apache/arrow-adbc/pull/1207#discussion_r1368981969


##########
docs/source/driver/snowflake.rst:
##########
@@ -406,6 +406,15 @@ These options map 1:1 with the Snowflake `Config object 
<https://pkg.go.dev/gith
     private key to be read in and parsed. Commonly encoded in PEM blocks
     of type "RSA PRIVATE KEY".
 
+``adbc.snowflake.sql.client_option.jwt_private_key_pkcs8_value``
+    Parses an encrypted or unencrypted PKCS #8 private key without having to
+    it from the file system. If using encrypted, the

Review Comment:
   ```suggestion
       Parses an encrypted or unencrypted PKCS #8 private key without having to
       read it from the file system. If using encrypted, the
   ```



##########
go/adbc/driver/snowflake/snowflake_database.go:
##########
@@ -335,6 +338,37 @@ func (d *databaseImpl) SetOptions(cnOptions 
map[string]string) error {
                                        Code: adbc.StatusInvalidArgument,
                                }
                        }
+               case OptionJwtPrivateKeyPkcs8Value:
+                       block, _ := pem.Decode([]byte(v))
+
+                       if block == nil {
+                               panic("Failed to parse PEM block containing the 
private key")
+                       }
+
+                       var parsedKey any
+
+                       if block.Type == "ENCRYPTED PRIVATE KEY" {
+                               passcode, ok := 
cnOptions[OptionJwtPrivateKeyPkcs8Password]
+                               if ok {
+                                       parsedKey, err = 
pkcs8.ParsePKCS8PrivateKey(block.Bytes, []byte(passcode))
+                               } else {
+                                       panic(OptionJwtPrivateKeyPkcs8Password 
+ " is not configured")
+                               }
+                       } else if block.Type == "PRIVATE KEY" {
+                               parsedKey, err = 
pkcs8.ParsePKCS8PrivateKey(block.Bytes)
+                       } else {
+                               panic(block.Type + " is not supported")
+                       }
+
+                       if err != nil {
+                               return adbc.Error{
+                                       Msg:  "failed parsing PKCS8 private 
key: " + err.Error(),

Review Comment:
   ```suggestion
                                        Msg:  "[Snowflake] failed parsing PKCS8 
private key: " + err.Error(),
   ```



##########
go/adbc/driver/snowflake/snowflake_database.go:
##########
@@ -335,6 +338,37 @@ func (d *databaseImpl) SetOptions(cnOptions 
map[string]string) error {
                                        Code: adbc.StatusInvalidArgument,
                                }
                        }
+               case OptionJwtPrivateKeyPkcs8Value:
+                       block, _ := pem.Decode([]byte(v))
+
+                       if block == nil {
+                               panic("Failed to parse PEM block containing the 
private key")
+                       }
+
+                       var parsedKey any
+
+                       if block.Type == "ENCRYPTED PRIVATE KEY" {
+                               passcode, ok := 
cnOptions[OptionJwtPrivateKeyPkcs8Password]
+                               if ok {
+                                       parsedKey, err = 
pkcs8.ParsePKCS8PrivateKey(block.Bytes, []byte(passcode))
+                               } else {
+                                       panic(OptionJwtPrivateKeyPkcs8Password 
+ " is not configured")

Review Comment:
   return an error, don't panic



##########
csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverImporter.cs:
##########
@@ -941,7 +941,6 @@ private unsafe void TranslateCode(AdbcStatusCode statusCode)
 
                     Dispose();
 
-                    throw new AdbcException(message);

Review Comment:
   Don't we need to throw still?



##########
go/adbc/driver/snowflake/snowflake_database.go:
##########
@@ -335,6 +338,37 @@ func (d *databaseImpl) SetOptions(cnOptions 
map[string]string) error {
                                        Code: adbc.StatusInvalidArgument,
                                }
                        }
+               case OptionJwtPrivateKeyPkcs8Value:
+                       block, _ := pem.Decode([]byte(v))
+
+                       if block == nil {
+                               panic("Failed to parse PEM block containing the 
private key")
+                       }
+
+                       var parsedKey any
+
+                       if block.Type == "ENCRYPTED PRIVATE KEY" {
+                               passcode, ok := 
cnOptions[OptionJwtPrivateKeyPkcs8Password]
+                               if ok {
+                                       parsedKey, err = 
pkcs8.ParsePKCS8PrivateKey(block.Bytes, []byte(passcode))
+                               } else {
+                                       panic(OptionJwtPrivateKeyPkcs8Password 
+ " is not configured")
+                               }
+                       } else if block.Type == "PRIVATE KEY" {
+                               parsedKey, err = 
pkcs8.ParsePKCS8PrivateKey(block.Bytes)
+                       } else {
+                               panic(block.Type + " is not supported")

Review Comment:
   ditto



##########
go/adbc/driver/snowflake/driver_test.go:
##########
@@ -672,3 +673,89 @@ func (suite *SnowflakeTests) TestUseHighPrecision() {
        suite.Equal(1234567.89, rec.Column(1).(*array.Float64).Value(0))
        suite.Equal(9876543210.99, rec.Column(1).(*array.Float64).Value(1))
 }
+
+func TestJwtAuthenticationUnencryptedValue(t *testing.T) {
+       // test doesn't participate in SnowflakeTests because
+       // JWT auth has a different behavior
+       uri, ok := os.LookupEnv("SNOWFLAKE_URI")
+       if !ok {
+               t.Skip("Cannot find the `SNOWFLAKE_URI` value")
+       }
+
+       keyValue, ok := os.LookupEnv("SNOWFLAKE_TEST_PKCS8_VALUE")
+       if !ok {
+               t.Skip("Cannot find the `SNOWFLAKE_TEST_PKCS8_VALUE` value")
+       }
+
+       ConnectWithJwt(uri, keyValue, "")
+}
+
+func TestJwtAuthenticationEncryptedValue(t *testing.T) {
+       // test doesn't participate in SnowflakeTests because
+       // JWT auth has a different behavior
+       uri, ok := os.LookupEnv("SNOWFLAKE_URI")
+       if !ok {
+               t.Skip("Cannot find the `SNOWFLAKE_URI` value")
+       }
+
+       keyValue, ok := os.LookupEnv("SNOWFLAKE_TEST_PKCS8_EN_VALUE")
+       if !ok {
+               t.Skip("Cannot find the `SNOWFLAKE_TEST_PKCS8_EN_VALUE` value")
+       }
+
+       passcode, ok := os.LookupEnv("SNOWFLAKE_TEST_PKCS8_PASS")
+       if !ok {
+               t.Skip("Cannot find the `SNOWFLAKE_TEST_PKCS8_PASS` value")
+       }
+
+       ConnectWithJwt(uri, keyValue, passcode)
+}
+
+func ConnectWithJwt(uri, keyValue, passcode string) {
+
+       // Windows funkiness
+       if runtime.GOOS == "windows" {
+               keyValue = strings.ReplaceAll(keyValue, "\\r", "\r")
+               keyValue = strings.ReplaceAll(keyValue, "\\n", "\n")
+       }

Review Comment:
   Hmm, how did this happen?



##########
go/adbc/driver/snowflake/snowflake_database.go:
##########
@@ -30,6 +32,7 @@ import (
        "github.com/apache/arrow-adbc/go/adbc"
        "github.com/apache/arrow-adbc/go/adbc/driver/driverbase"
        "github.com/snowflakedb/gosnowflake"
+       "github.com/youmark/pkcs8"

Review Comment:
   We need to regenerate the license for this, I believe



##########
go/adbc/driver/snowflake/driver_test.go:
##########
@@ -677,87 +674,88 @@ func (suite *SnowflakeTests) TestUseHighPrecision() {
        suite.Equal(9876543210.99, rec.Column(1).(*array.Float64).Value(1))
 }
 
-func TestUnencryptedJwtAuthenticationUsingString(t *testing.T) {
-       drv := gosnowflake.SnowflakeDriver{}
-
+func TestJwtAuthenticationUnencryptedValue(t *testing.T) {
+       // test doesn't participate in SnowflakeTests because of the
+       // JWT auth has a different behavior
        uri, ok := os.LookupEnv("SNOWFLAKE_URI")
-
        if !ok {
-               panic("Cannot find the `SNOWFLAKE_URI` value")
+               t.Skip("Cannot find the `SNOWFLAKE_URI` value")
        }
 
-       cfg, _ := gosnowflake.ParseDSN(uri)
-       cfg.Authenticator = gosnowflake.AuthTypeJwt
-
        keyValue, ok := os.LookupEnv("SNOWFLAKE_TEST_PKCS8_VALUE")

Review Comment:
   If we're not going to resolve this here can we file a followup?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to