Skip to content

Commit

Permalink
Revert "Fixed RD-12458: Add awsCredential parameter to S3.Build (#531)…
Browse files Browse the repository at this point in the history
…" (#536)

This reverts commit 34ddcf5.
  • Loading branch information
bgaidioz authored Nov 15, 2024
1 parent ea943aa commit ccec888
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 156 deletions.
2 changes: 1 addition & 1 deletion project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ object Dependencies {

val protocolRaw = "com.raw-labs" %% "protocol-raw" % "0.50.0"

val protocolCompiler = "com.raw-labs" %% "protocol-compiler" % "0.51.0"
val protocolCompiler = "com.raw-labs" %% "protocol-compiler" % "0.50.1"

val scalaLogging = "com.typesafe.scala-logging" %% "scala-logging" % "3.9.5"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,14 @@ import com.rawlabs.snapi.compiler._
import com.rawlabs.snapi.frontend.base.source.{BaseProgram, Type}
import com.rawlabs.snapi.frontend.inferrer.local.LocalInferrerTestContext
import com.rawlabs.protocol.compiler.{
AwsConfig,
DropboxAccessTokenConfig,
HttpHeadersConfig,
LocationConfig,
MySqlConfig,
OracleConfig,
PostgreSQLConfig,
S3AccessSecretKey,
S3Config,
SQLServerConfig,
SnowflakeConfig
}
Expand Down Expand Up @@ -85,17 +86,42 @@ object TestCredentials {

// Bucket with public access
val UnitTestPublicBucket = "rawlabs-public-test-data"
val UnitTestPublicBucketCred = S3Config.newBuilder().setRegion("eu-west-1").build()

// IAM user 'unit-test-private-bucket', which only has permissions only to access bucket 'rawlabs-private-test-data'
val UnitTestPrivateBucket = "rawlabs-private-test-data"
val UnitTestPrivateBucketCred = S3Config
.newBuilder()
.setAccessSecretKey(S3AccessSecretKey.newBuilder().setAccessKey(accessKeyId).setSecretKey(secretKeyId))
.setRegion("eu-west-1")
.build()

val UnitTestPrivateBucket2 = "rawlabs-unit-tests"
val UnitTestPrivateBucket2Cred = S3Config
.newBuilder()
.setAccessSecretKey(S3AccessSecretKey.newBuilder().setAccessKey(accessKeyId).setSecretKey(secretKeyId))
.setRegion("eu-west-1")
.build()

val UnitTestEmptyBucketPrivateBucket = "rawlabs-unit-test-empty-bucket"
val UnitTestEmptyBucketPrivateBucketCred = S3Config
.newBuilder()
.setAccessSecretKey(S3AccessSecretKey.newBuilder().setAccessKey(accessKeyId).setSecretKey(secretKeyId))
.setRegion("eu-west-1")
.build()

val UnitTestListRootPrivateBucket = "rawlabs-unit-test-list-root"
val unitTestPrivateBucketUsEast1 = "rawlabs-unit-tests-us-east-1"
val UnitTestListRootPrivateBucketCred = S3Config
.newBuilder()
.setAccessSecretKey(S3AccessSecretKey.newBuilder().setAccessKey(accessKeyId).setSecretKey(secretKeyId))
.setRegion("eu-west-1")
.build()

val rawAwsCredentials = AwsConfig
val unitTestPrivateBucketUsEast1 = "rawlabs-unit-tests-us-east-1"
val unitTestPrivateBucketUsEast1Cred = S3Config
.newBuilder()
.setAccessKey(accessKeyId)
.setSecretKey(secretKeyId)
.setAccessSecretKey(S3AccessSecretKey.newBuilder().setAccessKey(accessKeyId).setSecretKey(secretKeyId))
.setRegion("us-east-1")
.build()

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -207,8 +233,8 @@ trait SnapiTestContext
locationConfigs.put(name, locationConfig)
}

def awsCreds(name: String, awsConfig: AwsConfig): Unit = {
locationConfig(name, LocationConfig.newBuilder().setAws(awsConfig).build())
def s3Bucket(name: String, bucket: S3Config): Unit = {
locationConfig(name, LocationConfig.newBuilder().setS3(bucket).build())
}

def rdbms(name: String, db: MySqlConfig): Unit = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class S3PackageTest extends SnapiTestContext {

import com.rawlabs.snapi.compiler.tests.TestCredentials._

awsCreds("raw-aws", rawAwsCredentials)
s3Bucket(UnitTestPrivateBucket, UnitTestPrivateBucketCred)
// Reading a public bucket without credentials
test(s"""let
| data = Csv.InferAndRead(
Expand Down Expand Up @@ -50,16 +50,16 @@ class S3PackageTest extends SnapiTestContext {
| S3.Build(
| "$UnitTestPrivateBucket",
| "/students.csv",
| region = "eu-west-1",
| accessKey = "${rawAwsCredentials.getAccessKey}",
| secretKey = "${rawAwsCredentials.getSecretKey}"
| region = "${UnitTestPrivateBucketCred.getRegion}",
| accessKey = "${UnitTestPrivateBucketCred.getAccessSecretKey.getAccessKey}",
| secretKey = "${UnitTestPrivateBucketCred.getAccessSecretKey.getSecretKey}"
| )
| )
|in
| Collection.Count(data)
|""".stripMargin)(it => it should evaluateTo("7"))

// Reading a private bucket without credential
// Reading a private bucket using a registered credential
test(s"""let
| data = Csv.InferAndRead(
| S3.Build(
Expand All @@ -69,17 +69,11 @@ class S3PackageTest extends SnapiTestContext {
| )
|in
| Collection.Count(data)
|""".stripMargin)(it => it should runErrorAs("path not authorized"))
|""".stripMargin)(it => it should evaluateTo("7"))

// Reading a private bucket using a registered credential
// Using the automatic casting from url to S3Location using credentials
test(s"""let
| data = Csv.InferAndRead(
| S3.Build(
| "$UnitTestPrivateBucket",
| "/students.csv",
| awsCredential = "raw-aws"
| )
| )
| data = Csv.InferAndRead("s3://$UnitTestPrivateBucket/students.csv")
|in
| Collection.Count(data)
|""".stripMargin)(it => it should evaluateTo("7"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ class LocationPackageTest extends SnapiTestContext {
import com.rawlabs.snapi.compiler.tests.TestCredentials._

httpHeaders("dropbox-token", Map("Authorization" -> ("Bearer " + dropboxLongLivedAccessToken)))
awsCreds("raw-aws", rawAwsCredentials)

s3Bucket(UnitTestPrivateBucket2, UnitTestPrivateBucket2Cred)

property("raw.utils.sources.dropbox.clientId", dropboxClientId)

Expand All @@ -31,22 +32,6 @@ class LocationPackageTest extends SnapiTestContext {
| )
|)""".stripMargin)(it => it should run)

test("""
|String.Read(
| Http.Post(
| "https://api.dropboxapi.com/2/users/get_space_usage",
| authCredentialName = "dropboxToken" // wrong: doesn't exist
| )
|)""".stripMargin)(it => it should runErrorAs("unknown credential: dropboxToken"))

test("""
|String.Read(
| Http.Post(
| "https://api.dropboxapi.com/2/users/get_space_usage",
| authCredentialName = "raw-aws" // wrong: AWS credential
| )
|)""".stripMargin)(it => it should runErrorAs("credential is not an HTTP headers"))

// reading a public s3 bucket without registering or passing credentials
test(s"""let
| data = Csv.InferAndRead("s3://$UnitTestPublicBucket/students.csv")
Expand Down Expand Up @@ -101,24 +86,24 @@ class LocationPackageTest extends SnapiTestContext {
| S3.Build(
| "$UnitTestPrivateBucket",
| "students.csv",
| region = "eu-west-1",
| accessKey = "${rawAwsCredentials.getAccessKey}",
| secretKey = "${rawAwsCredentials.getSecretKey}"
| region = "${UnitTestPrivateBucketCred.getRegion}",
| accessKey = "${UnitTestPrivateBucketCred.getAccessSecretKey.getAccessKey}",
| secretKey = "${UnitTestPrivateBucketCred.getAccessSecretKey.getSecretKey}"
| )
| )
|in
| Collection.Count(data)
|""".stripMargin)(it => it should evaluateTo("7"))

// using a private bucket registered in the credentials server
test(s"""String.Read(S3.Build("$UnitTestPrivateBucket2", "/file1.csv", awsCredential = "raw-aws"))
test(s"""String.Read(S3.Build("$UnitTestPrivateBucket2", "/file1.csv"))
|""".stripMargin)(it => it should evaluateTo(""" "foobar" """))

test(s"""let dir = S3.Build(
| "$UnitTestPrivateBucket", "/publications/publications-hjson/*.json",
| region = "eu-west-1",
| accessKey = "${rawAwsCredentials.getAccessKey}",
| secretKey = "${rawAwsCredentials.getSecretKey}"
| region = "${UnitTestPrivateBucketCred.getRegion}",
| accessKey = "${UnitTestPrivateBucketCred.getAccessSecretKey.getAccessKey}",
| secretKey = "${UnitTestPrivateBucketCred.getAccessSecretKey.getSecretKey}"
| ),
| files = Location.Ls(dir),
| lines = List.Unnest(files, f -> List.From(String.ReadLines(f)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,25 @@ class S3PackageTest extends SnapiTestContext {

import TestCredentials._

awsCreds("raw-aws", rawAwsCredentials)
s3Bucket(UnitTestPrivateBucket2, UnitTestPrivateBucket2Cred)

// reading a non public s3 bucket passing credentials in the location settings
test(s"""let
| data = Csv.InferAndRead(
| S3.Build(
| "$UnitTestPrivateBucket",
| "/students.csv",
| region = "eu-west-1",
| accessKey = "${rawAwsCredentials.getAccessKey}",
| secretKey = "${rawAwsCredentials.getSecretKey}"
| region = "${UnitTestPrivateBucketCred.getRegion}",
| accessKey = "${UnitTestPrivateBucketCred.getAccessSecretKey.getAccessKey}",
| secretKey = "${UnitTestPrivateBucketCred.getAccessSecretKey.getSecretKey}"
| )
| )
|in
| Collection.Count(data)
|""".stripMargin)(it => it should evaluateTo("7"))

// using a private bucket registered in the credentials server
test(s"""String.Read(S3.Build("$UnitTestPrivateBucket2", "/file1.csv", awsCredential = "raw-aws"))
test(s"""String.Read(S3.Build("$UnitTestPrivateBucket2", "/file1.csv"))
|""".stripMargin)(it => it should evaluateTo(""" "foobar" """))

// listing a s3 bucket from us-east-1 (non default region)
Expand All @@ -46,26 +46,9 @@ class S3PackageTest extends SnapiTestContext {
| S3.Build(
| "$unitTestPrivateBucketUsEast1",
| "/csvs/01",
| region = "us-east-1",
| accessKey = "${rawAwsCredentials.getAccessKey}",
| secretKey = "${rawAwsCredentials.getSecretKey}"
| )
| )
|in
| data.url
|""".stripMargin)(it => it should evaluateTo("""[
| "s3://rawlabs-unit-tests-us-east-1/csvs/01/data2.csv",
| "s3://rawlabs-unit-tests-us-east-1/csvs/01/data1.csv"
|]""".stripMargin))

// listing a s3 bucket from us-east-1 (non default region, using the credentials name)
test(s"""let
| data = Location.Ll(
| S3.Build(
| "$unitTestPrivateBucketUsEast1",
| "/csvs/01",
| region = "us-east-1",
| awsCredential = "raw-aws"
| region = "${unitTestPrivateBucketUsEast1Cred.getRegion}",
| accessKey = "${unitTestPrivateBucketUsEast1Cred.getAccessSecretKey.getAccessKey}",
| secretKey = "${unitTestPrivateBucketUsEast1Cred.getAccessSecretKey.getSecretKey}"
| )
| )
|in
Expand All @@ -81,24 +64,8 @@ class S3PackageTest extends SnapiTestContext {
| S3.Build(
| "$unitTestPrivateBucketUsEast1",
| "/csvs/01",
| accessKey = "${rawAwsCredentials.getAccessKey}",
| secretKey = "${rawAwsCredentials.getSecretKey}"
| )
| )
|in
| data.url
|""".stripMargin)(it => it should evaluateTo("""[
| "s3://rawlabs-unit-tests-us-east-1/csvs/01/data2.csv",
| "s3://rawlabs-unit-tests-us-east-1/csvs/01/data1.csv"
|]""".stripMargin))

// listing a s3 bucket from us-east-1 without passing the region (using the credentials name)
test(s"""let
| data = Location.Ll(
| S3.Build(
| "$unitTestPrivateBucketUsEast1",
| "/csvs/01",
| awsCredential = "raw-aws"
| accessKey = "${unitTestPrivateBucketUsEast1Cred.getAccessSecretKey.getAccessKey}",
| secretKey = "${unitTestPrivateBucketUsEast1Cred.getAccessSecretKey.getSecretKey}"
| )
| )
|in
Expand All @@ -108,17 +75,4 @@ class S3PackageTest extends SnapiTestContext {
| "s3://rawlabs-unit-tests-us-east-1/csvs/01/data1.csv"
|]""".stripMargin))

// error with wrong credential name
test(s"""let
| data = Location.Ll(
| S3.Build(
| "$unitTestPrivateBucketUsEast1",
| "/csvs/01",
| awsCredential = "private.data.us" // a typo
| )
| )
|in
| data.url
|""".stripMargin)(it => it should runErrorAs("unknown credential: private.data.us"))

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ import com.rawlabs.snapi.compiler.tests.SnapiTestContext

class RD5932Test extends SnapiTestContext {

awsCreds("private-creds", TestCredentials.rawAwsCredentials)
s3Bucket(TestCredentials.UnitTestPrivateBucket, TestCredentials.UnitTestPrivateBucketCred)

test(
"""Json.InferAndRead(S3.Build("rawlabs-private-test-data", "rd-5932.json", awsCredential = "private-creds"))"""
) {
test("""Json.InferAndRead("s3://rawlabs-private-test-data/rd-5932.json")""") {
_ should run
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -604,19 +604,42 @@ object LocationDescription {
val userInfoParts = uriUserInfo.split(":")
maybeAccessKey = Some(userInfoParts(0))
if (maybeAccessKey.get.isEmpty) {
return Left("missing AWS access key")
return Left("missing S3 access key")
}
if (userInfoParts.length > 1) {
maybeSecretKey = Some(userInfoParts(1))
if (maybeSecretKey.get.isEmpty) {
return Left("missing AWS secret key")
return Left("missing S3 secret key")
}
} else {
return Left("missing AWS secret key")
return Left("missing S3 secret key")
}
}
// TODO (msb): There is no way to specify the region when using a direct URL...
Right(S3PathLocationDescription(bucketName, None, maybeAccessKey, maybeSecretKey, objectKey))

if (maybeAccessKey.isEmpty) {
// If the access key/secret key are not defined, check if credential exists.
programEnvironment.locationConfigs.get(bucketName) match {
case Some(l) if l.hasS3 =>
val s3Credential = l.getS3
Right(
S3PathLocationDescription(
bucketName,
if (s3Credential.hasRegion) Some(s3Credential.getRegion) else None,
if (s3Credential.hasAccessSecretKey) Some(s3Credential.getAccessSecretKey.getAccessKey) else None,
if (s3Credential.hasAccessSecretKey) Some(s3Credential.getAccessSecretKey.getSecretKey) else None,
objectKey
)
)
case Some(l) if l.hasError => Left(l.getError.getMessage)
case Some(_) => Left("not a S3 credential")
case None =>
// Anonymous access.
Right(S3PathLocationDescription(bucketName, None, None, None, objectKey))
}
} else {
// TODO (msb): There is no way to specify the region when using a direct URL...
Right(S3PathLocationDescription(bucketName, None, maybeAccessKey, maybeSecretKey, objectKey))
}
case "dropbox" =>
// In Dropbox, the host is the name of the credential
val DROPBOX_REGEX(name, path) = url
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@ class S3BuildEntry extends EntryExtension {
params = List(
ParamDoc("bucket", TypeDoc(List("string")), "The name of the bucket."),
ParamDoc("path", TypeDoc(List("string")), "The path to the file in the bucket."),
ParamDoc(
"awsCredential",
TypeDoc(List("string")),
"The name of the AWS credential registered in the credentials storage.",
isOptional = true
),
ParamDoc("region", TypeDoc(List("string")), "The region of the bucket, e.g. 'eu-west-1'.", isOptional = true),
ParamDoc("accessKey", TypeDoc(List("string")), "The AWS access key.", isOptional = true),
ParamDoc("secretKey", TypeDoc(List("string")), "The AWS secret key.", isOptional = true)
Expand All @@ -66,10 +60,9 @@ class S3BuildEntry extends EntryExtension {
Right(ExpParam(SnapiStringType()))
}

override def optionalParams: Option[Set[String]] = Some(Set("region", "accessKey", "secretKey", "awsCredential"))
override def optionalParams: Option[Set[String]] = Some(Set("region", "accessKey", "secretKey"))

override def getOptionalParam(prevMandatoryArgs: Seq[Arg], idn: String): Either[String, Param] = idn match {
case "awsCredential" => Right(ExpParam(SnapiStringType()))
case "region" => Right(ExpParam(SnapiStringType()))
case "accessKey" => Right(ExpParam(SnapiStringType()))
case "secretKey" => Right(ExpParam(SnapiStringType()))
Expand Down
Loading

0 comments on commit ccec888

Please sign in to comment.