From 3d285dc94d5fc8d073a1b89c44bf2026a9c2000c Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Thu, 12 Sep 2024 13:03:16 +0200 Subject: [PATCH 1/4] Core: Add credentials to loadTable / loadView responses --- .../rest/credentials/AdlsCredential.java | 34 +++ .../iceberg/rest/credentials/Credential.java | 21 ++ .../rest/credentials/CredentialParser.java | 115 +++++++++ .../rest/credentials/GcsCredential.java | 34 +++ .../rest/credentials/S3Credential.java | 38 +++ .../rest/responses/LoadTableResponse.java | 29 ++- .../responses/LoadTableResponseParser.java | 24 ++ .../rest/responses/LoadViewResponse.java | 8 + .../responses/LoadViewResponseParser.java | 24 ++ .../credentials/TestCredentialParser.java | 236 ++++++++++++++++++ .../TestLoadTableResponseParser.java | 208 +++++++++++++++ .../responses/TestLoadViewResponseParser.java | 182 ++++++++++++++ 12 files changed, 951 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java create mode 100644 core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java new file mode 100644 index 000000000000..df3c51a67d77 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.credentials; + +import java.time.Instant; +import org.immutables.value.Value; + +@Value.Immutable +public interface AdlsCredential extends Credential { + @Value.Default + default String scheme() { + return "afbs"; + } + + String sasToken(); + + Instant expiresAt(); +} diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java new file mode 100644 index 000000000000..74fa3218c209 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java @@ -0,0 +1,21 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.credentials; + +public interface Credential {} diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java b/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java new file mode 100644 index 000000000000..c9ed8085db0f --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.credentials; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonNode; +import java.io.IOException; +import java.time.Instant; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.util.JsonUtil; + +public class CredentialParser { + private static final String TYPE = "type"; + private static final String S3 = "s3"; + private static final String GCS = "gcs"; + private static final String ADLS = "adls"; + private static final String ACCESS_KEY_ID = "access-key-id"; + private static final String SECRET_ACCESS_KEY = "secret-access-key"; + private static final String GCS_TOKEN = "token"; + private static final String ADLS_TOKEN = "sas-token"; + private static final String S3_TOKEN = "session-token"; + private static final String EXPIRES_AT = "expires-at-ms"; + private static final String SCHEME = "scheme"; + + private CredentialParser() {} + + public static String toJson(Credential credentials) { + return toJson(credentials, false); + } + + public static String toJson(Credential credentials, boolean pretty) { + return JsonUtil.generate(gen -> toJson(credentials, gen), pretty); + } + + public static void toJson(Credential credentials, JsonGenerator gen) throws IOException { + Preconditions.checkArgument(null != credentials, "Invalid credential: null"); + + gen.writeStartObject(); + + if (credentials instanceof S3Credential) { + S3Credential s3 = (S3Credential) credentials; + gen.writeStringField(TYPE, S3); + gen.writeStringField(SCHEME, s3.scheme()); + gen.writeStringField(ACCESS_KEY_ID, s3.accessKeyId()); + gen.writeStringField(SECRET_ACCESS_KEY, s3.secretAccessKey()); + gen.writeStringField(S3_TOKEN, s3.sessionToken()); + gen.writeNumberField(EXPIRES_AT, s3.expiresAt().toEpochMilli()); + } else if (credentials instanceof GcsCredential) { + GcsCredential gcs = (GcsCredential) credentials; + gen.writeStringField(TYPE, GCS); + gen.writeStringField(SCHEME, gcs.scheme()); + gen.writeStringField(GCS_TOKEN, gcs.token()); + gen.writeNumberField(EXPIRES_AT, gcs.expiresAt().toEpochMilli()); + } else if (credentials instanceof AdlsCredential) { + AdlsCredential adls = (AdlsCredential) credentials; + gen.writeStringField(TYPE, ADLS); + gen.writeStringField(SCHEME, adls.scheme()); + gen.writeStringField(ADLS_TOKEN, adls.sasToken()); + gen.writeNumberField(EXPIRES_AT, adls.expiresAt().toEpochMilli()); + } + + gen.writeEndObject(); + } + + public static Credential fromJson(String json) { + return JsonUtil.parse(json, CredentialParser::fromJson); + } + + public static Credential fromJson(JsonNode json) { + Preconditions.checkArgument(null != json, "Cannot parse credential from null object"); + String credentialType = JsonUtil.getString(TYPE, json); + String scheme = JsonUtil.getString(SCHEME, json); + switch (credentialType) { + case S3: + return ImmutableS3Credential.builder() + .scheme(scheme) + .accessKeyId(JsonUtil.getString(ACCESS_KEY_ID, json)) + .secretAccessKey(JsonUtil.getString(SECRET_ACCESS_KEY, json)) + .sessionToken(JsonUtil.getString(S3_TOKEN, json)) + .expiresAt(Instant.ofEpochMilli(JsonUtil.getLong(EXPIRES_AT, json))) + .build(); + case GCS: + return ImmutableGcsCredential.builder() + .scheme(scheme) + .token(JsonUtil.getString(GCS_TOKEN, json)) + .expiresAt(Instant.ofEpochMilli(JsonUtil.getLong(EXPIRES_AT, json))) + .build(); + case ADLS: + return ImmutableAdlsCredential.builder() + .scheme(scheme) + .sasToken(JsonUtil.getString(ADLS_TOKEN, json)) + .expiresAt(Instant.ofEpochMilli(JsonUtil.getLong(EXPIRES_AT, json))) + .build(); + + default: + return null; + } + } +} diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java new file mode 100644 index 000000000000..d2005f284a25 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.credentials; + +import java.time.Instant; +import org.immutables.value.Value; + +@Value.Immutable +public interface GcsCredential extends Credential { + @Value.Default + default String scheme() { + return "gs"; + } + + String token(); + + Instant expiresAt(); +} diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java new file mode 100644 index 000000000000..f1ac0fd1a20a --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java @@ -0,0 +1,38 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.credentials; + +import java.time.Instant; +import org.immutables.value.Value; + +@Value.Immutable +public interface S3Credential extends Credential { + @Value.Default + default String scheme() { + return "s3"; + } + + String accessKeyId(); + + String secretAccessKey(); + + String sessionToken(); + + Instant expiresAt(); +} diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java index 519d1fc34044..d52cdbbc253a 100644 --- a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java +++ b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java @@ -18,13 +18,17 @@ */ package org.apache.iceberg.rest.responses; +import java.util.List; import java.util.Map; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.relocated.com.google.common.base.MoreObjects; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.rest.RESTResponse; +import org.apache.iceberg.rest.credentials.Credential; /** * A REST response that is used when a table is successfully loaded. @@ -40,16 +44,21 @@ public class LoadTableResponse implements RESTResponse { private TableMetadata metadata; private Map config; private TableMetadata metadataWithLocation; + private List credentials; public LoadTableResponse() { // Required for Jackson deserialization } private LoadTableResponse( - String metadataLocation, TableMetadata metadata, Map config) { + String metadataLocation, + TableMetadata metadata, + Map config, + List credentials) { this.metadataLocation = metadataLocation; this.metadata = metadata; this.config = config; + this.credentials = credentials; } @Override @@ -74,12 +83,17 @@ public Map config() { return config != null ? config : ImmutableMap.of(); } + public List credentials() { + return credentials != null ? credentials : ImmutableList.of(); + } + @Override public String toString() { return MoreObjects.toStringHelper(this) .add("metadataLocation", metadataLocation) .add("metadata", metadata) .add("config", config) + .add("credentials", credentials) .toString(); } @@ -91,6 +105,7 @@ public static class Builder { private String metadataLocation; private TableMetadata metadata; private final Map config = Maps.newHashMap(); + private final List credentials = Lists.newArrayList(); private Builder() {} @@ -110,9 +125,19 @@ public Builder addAllConfig(Map properties) { return this; } + public Builder addCredential(Credential credential) { + credentials.add(credential); + return this; + } + + public Builder addAllCredentials(List credentialsToAdd) { + credentials.addAll(credentialsToAdd); + return this; + } + public LoadTableResponse build() { Preconditions.checkNotNull(metadata, "Invalid metadata: null"); - return new LoadTableResponse(metadataLocation, metadata, config); + return new LoadTableResponse(metadataLocation, metadata, config, credentials); } } } diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponseParser.java b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponseParser.java index 316c5160ddc5..9a161801a877 100644 --- a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponseParser.java +++ b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponseParser.java @@ -21,9 +21,12 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; +import java.util.Optional; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.rest.credentials.Credential; +import org.apache.iceberg.rest.credentials.CredentialParser; import org.apache.iceberg.util.JsonUtil; public class LoadTableResponseParser { @@ -31,6 +34,7 @@ public class LoadTableResponseParser { private static final String METADATA_LOCATION = "metadata-location"; private static final String METADATA = "metadata"; private static final String CONFIG = "config"; + private static final String STORAGE_CREDENTIALS = "storage-credentials"; private LoadTableResponseParser() {} @@ -58,6 +62,15 @@ public static void toJson(LoadTableResponse response, JsonGenerator gen) throws JsonUtil.writeStringMap(CONFIG, response.config(), gen); } + if (!response.credentials().isEmpty()) { + gen.writeArrayFieldStart(STORAGE_CREDENTIALS); + for (Credential credential : response.credentials()) { + CredentialParser.toJson(credential, gen); + } + + gen.writeEndArray(); + } + gen.writeEndObject(); } @@ -85,6 +98,17 @@ public static LoadTableResponse fromJson(JsonNode json) { builder.addAllConfig(JsonUtil.getStringMap(CONFIG, json)); } + if (json.hasNonNull(STORAGE_CREDENTIALS)) { + JsonNode credentials = JsonUtil.get(STORAGE_CREDENTIALS, json); + Preconditions.checkArgument( + credentials.isArray(), "Cannot parse credentials from non-array: %s", credentials); + + credentials.forEach( + cred -> + Optional.ofNullable(CredentialParser.fromJson(cred)) + .ifPresent(builder::addCredential)); + } + return builder.build(); } } diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponse.java index d07ba872fdaa..d7f9040e77f7 100644 --- a/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponse.java +++ b/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponse.java @@ -18,8 +18,11 @@ */ package org.apache.iceberg.rest.responses; +import java.util.List; import java.util.Map; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.rest.RESTResponse; +import org.apache.iceberg.rest.credentials.Credential; import org.apache.iceberg.view.ViewMetadata; import org.immutables.value.Value; @@ -31,6 +34,11 @@ public interface LoadViewResponse extends RESTResponse { Map config(); + @Value.Default + default List credentials() { + return ImmutableList.of(); + } + @Override default void validate() { // nothing to validate as it's not possible to create an invalid instance diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java b/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java index a8aaf17e5d76..447b85b2b6bb 100644 --- a/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java +++ b/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java @@ -21,7 +21,10 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; +import java.util.Optional; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.rest.credentials.Credential; +import org.apache.iceberg.rest.credentials.CredentialParser; import org.apache.iceberg.util.JsonUtil; import org.apache.iceberg.view.ViewMetadata; import org.apache.iceberg.view.ViewMetadataParser; @@ -31,6 +34,7 @@ public class LoadViewResponseParser { private static final String METADATA_LOCATION = "metadata-location"; private static final String METADATA = "metadata"; private static final String CONFIG = "config"; + private static final String STORAGE_CREDENTIALS = "storage-credentials"; private LoadViewResponseParser() {} @@ -56,6 +60,15 @@ public static void toJson(LoadViewResponse response, JsonGenerator gen) throws I JsonUtil.writeStringMap(CONFIG, response.config(), gen); } + if (!response.credentials().isEmpty()) { + gen.writeArrayFieldStart(STORAGE_CREDENTIALS); + for (Credential credential : response.credentials()) { + CredentialParser.toJson(credential, gen); + } + + gen.writeEndArray(); + } + gen.writeEndObject(); } @@ -80,6 +93,17 @@ public static LoadViewResponse fromJson(JsonNode json) { builder.config(JsonUtil.getStringMap(CONFIG, json)); } + if (json.hasNonNull(STORAGE_CREDENTIALS)) { + JsonNode credentials = JsonUtil.get(STORAGE_CREDENTIALS, json); + Preconditions.checkArgument( + credentials.isArray(), "Cannot parse credentials from non-array: %s", credentials); + + credentials.forEach( + cred -> + Optional.ofNullable(CredentialParser.fromJson(cred)) + .ifPresent(builder::addCredentials)); + } + return builder.build(); } } diff --git a/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java b/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java new file mode 100644 index 000000000000..e9af460464d0 --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java @@ -0,0 +1,236 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.credentials; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.fasterxml.jackson.databind.JsonNode; +import java.time.Instant; +import org.junit.jupiter.api.Test; + +public class TestCredentialParser { + @Test + public void nullAndEmptyCheck() { + assertThatThrownBy(() -> CredentialParser.toJson(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid credential: null"); + + assertThatThrownBy(() -> CredentialParser.fromJson((JsonNode) null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse credential from null object"); + } + + @Test + public void invalidTypeOrMissingType() { + assertThat(CredentialParser.fromJson("{\"type\": \"unknown\",\"scheme\": \"s3\"}")).isNull(); + + assertThatThrownBy(() -> CredentialParser.fromJson("{}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: type"); + + assertThatThrownBy(() -> CredentialParser.fromJson("{\"x\": \"y\"}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: type"); + } + + @Test + public void s3Credential() { + Instant expiresAt = Instant.now(); + + Credential credential = + ImmutableS3Credential.builder() + .accessKeyId("keyId") + .scheme("s3://custom-uri") + .secretAccessKey("accessKey") + .sessionToken("sessionToken") + .expiresAt(expiresAt) + .build(); + + String expectedJson = + String.format( + "{\n" + + " \"type\" : \"s3\",\n" + + " \"scheme\" : \"s3://custom-uri\",\n" + + " \"access-key-id\" : \"keyId\",\n" + + " \"secret-access-key\" : \"accessKey\",\n" + + " \"session-token\" : \"sessionToken\",\n" + + " \"expires-at-ms\" : %s\n" + + "}", + expiresAt.toEpochMilli()); + + String json = CredentialParser.toJson(credential, true); + assertThat(json).isEqualTo(expectedJson); + assertThat(CredentialParser.toJson(CredentialParser.fromJson(json), true)) + .isEqualTo(expectedJson); + } + + @Test + public void s3CredentialWithMissingProperties() { + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\n" + + " \"type\" : \"s3\",\n" + + " \"access-key-id\" : \"keyId\",\n" + + " \"secret-access-key\" : \"accessKey\",\n" + + " \"session-token\" : \"sessionToken\"" + + "}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: scheme"); + + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\n" + + " \"type\" : \"s3\",\n" + + " \"scheme\" : \"s3\",\n" + + " \"access-key-id\" : \"keyId\",\n" + + " \"secret-access-key\" : \"accessKey\",\n" + + " \"session-token\" : \"sessionToken\"" + + "}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing long: expires-at-ms"); + + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\n" + + " \"type\" : \"s3\",\n" + + " \"scheme\" : \"s3\",\n" + + " \"access-key-id\" : \"keyId\",\n" + + " \"secret-access-key\" : \"accessKey\",\n" + + " \"expires-at-ms\" : \"1\"" + + "}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: session-token"); + + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\n" + + " \"type\" : \"s3\",\n" + + " \"scheme\" : \"s3\",\n" + + " \"access-key-id\" : \"keyId\",\n" + + " \"session-token\" : \"sessionToken\",\n" + + " \"expires-at-ms\" : \"1\"" + + "}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: secret-access-key"); + + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\n" + + " \"type\" : \"s3\",\n" + + " \"scheme\" : \"s3\",\n" + + " \"secret-access-key\" : \"accessKey\",\n" + + " \"expires-at-ms\" : \"1\"" + + "}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: access-key-id"); + } + + @Test + public void gcsCredential() { + Instant expiresAt = Instant.now(); + + Credential credential = + ImmutableGcsCredential.builder().token("gcsToken").expiresAt(expiresAt).build(); + + String expectedJson = + String.format( + "{\n" + + " \"type\" : \"gcs\",\n" + + " \"scheme\" : \"gs\",\n" + + " \"token\" : \"gcsToken\",\n" + + " \"expires-at-ms\" : %s\n" + + "}", + expiresAt.toEpochMilli()); + + String json = CredentialParser.toJson(credential, true); + assertThat(json).isEqualTo(expectedJson); + assertThat(CredentialParser.toJson(CredentialParser.fromJson(json), true)) + .isEqualTo(expectedJson); + } + + @Test + public void gcsCredentialWithMissingProperties() { + assertThatThrownBy(() -> CredentialParser.fromJson("{\"type\": \"gcs\"}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: scheme"); + + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\"type\": \"gcs\",\"scheme\":\"gs\",\"token\": \"gcsToken\"}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing long: expires-at-ms"); + + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\"type\": \"gcs\",\"scheme\":\"gs\",\"expires-at-ms\": 1}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: token"); + } + + @Test + public void adlsCredential() { + Instant expiresAt = Instant.now(); + + Credential credential = + ImmutableAdlsCredential.builder().sasToken("sasToken").expiresAt(expiresAt).build(); + + String expectedJson = + String.format( + "{\n" + + " \"type\" : \"adls\",\n" + + " \"scheme\" : \"afbs\",\n" + + " \"sas-token\" : \"sasToken\",\n" + + " \"expires-at-ms\" : %s\n" + + "}", + expiresAt.toEpochMilli()); + + String json = CredentialParser.toJson(credential, true); + assertThat(json).isEqualTo(expectedJson); + assertThat(CredentialParser.toJson(CredentialParser.fromJson(json), true)) + .isEqualTo(expectedJson); + } + + @Test + public void adlsCredentialWithMissingProperties() { + assertThatThrownBy(() -> CredentialParser.fromJson("{\"type\": \"adls\"}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: scheme"); + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\"type\": \"adls\",\"scheme\": \"afbs\",\"sas-token\": \"adlsToken\"}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing long: expires-at-ms"); + + assertThatThrownBy( + () -> + CredentialParser.fromJson( + "{\"type\": \"adls\",\"scheme\": \"afbs\",\"expires-at-ms\": 1}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing string: sas-token"); + } +} diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java index b87c66bffe94..4326d19e2496 100644 --- a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java @@ -22,11 +22,14 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.fasterxml.jackson.databind.JsonNode; +import java.time.Instant; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Schema; import org.apache.iceberg.SortOrder; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.rest.credentials.ImmutableAdlsCredential; +import org.apache.iceberg.rest.credentials.ImmutableGcsCredential; import org.apache.iceberg.types.Types; import org.junit.jupiter.api.Test; @@ -200,4 +203,209 @@ public void roundTripSerdeWithConfig() { assertThat(LoadTableResponseParser.toJson(LoadTableResponseParser.fromJson(json), true)) .isEqualTo(expectedJson); } + + @Test + public void roundTripSerdeWithCredentials() { + String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b"; + TableMetadata metadata = + TableMetadata.buildFromEmpty() + .assignUUID(uuid) + .setLocation("location") + .setCurrentSchema( + new Schema(Types.NestedField.required(1, "x", Types.LongType.get())), 1) + .addPartitionSpec(PartitionSpec.unpartitioned()) + .addSortOrder(SortOrder.unsorted()) + .discardChanges() + .withMetadataLocation("metadata-location") + .build(); + + LoadTableResponse response = + LoadTableResponse.builder() + .withTableMetadata(metadata) + .addAllConfig(ImmutableMap.of("key1", "val1", "key2", "val2")) + .addCredential( + ImmutableGcsCredential.builder() + .token("gcsToken") + .expiresAt(Instant.ofEpochMilli(1000)) + .build()) + .addCredential( + ImmutableGcsCredential.builder() + .scheme("gs://custom-uri") + .token("token") + .expiresAt(Instant.ofEpochMilli(1000)) + .build()) + .addCredential( + ImmutableAdlsCredential.builder() + .sasToken("sasToken") + .expiresAt(Instant.ofEpochMilli(1000)) + .build()) + .build(); + + String expectedJson = + String.format( + "{\n" + + " \"metadata-location\" : \"metadata-location\",\n" + + " \"metadata\" : {\n" + + " \"format-version\" : 2,\n" + + " \"table-uuid\" : \"386b9f01-002b-4d8c-b77f-42c3fd3b7c9b\",\n" + + " \"location\" : \"location\",\n" + + " \"last-sequence-number\" : 0,\n" + + " \"last-updated-ms\" : %s,\n" + + " \"last-column-id\" : 1,\n" + + " \"current-schema-id\" : 0,\n" + + " \"schemas\" : [ {\n" + + " \"type\" : \"struct\",\n" + + " \"schema-id\" : 0,\n" + + " \"fields\" : [ {\n" + + " \"id\" : 1,\n" + + " \"name\" : \"x\",\n" + + " \"required\" : true,\n" + + " \"type\" : \"long\"\n" + + " } ]\n" + + " } ],\n" + + " \"default-spec-id\" : 0,\n" + + " \"partition-specs\" : [ {\n" + + " \"spec-id\" : 0,\n" + + " \"fields\" : [ ]\n" + + " } ],\n" + + " \"last-partition-id\" : 999,\n" + + " \"default-sort-order-id\" : 0,\n" + + " \"sort-orders\" : [ {\n" + + " \"order-id\" : 0,\n" + + " \"fields\" : [ ]\n" + + " } ],\n" + + " \"properties\" : { },\n" + + " \"current-snapshot-id\" : -1,\n" + + " \"refs\" : { },\n" + + " \"snapshots\" : [ ],\n" + + " \"statistics\" : [ ],\n" + + " \"partition-statistics\" : [ ],\n" + + " \"snapshot-log\" : [ ],\n" + + " \"metadata-log\" : [ ]\n" + + " },\n" + + " \"config\" : {\n" + + " \"key1\" : \"val1\",\n" + + " \"key2\" : \"val2\"\n" + + " },\n" + + " \"storage-credentials\" : [ {\n" + + " \"type\" : \"gcs\",\n" + + " \"scheme\" : \"gs\",\n" + + " \"token\" : \"gcsToken\",\n" + + " \"expires-at-ms\" : 1000\n" + + " }, {\n" + + " \"type\" : \"gcs\",\n" + + " \"scheme\" : \"gs://custom-uri\",\n" + + " \"token\" : \"token\",\n" + + " \"expires-at-ms\" : 1000\n" + + " }, {\n" + + " \"type\" : \"adls\",\n" + + " \"scheme\" : \"afbs\",\n" + + " \"sas-token\" : \"sasToken\",\n" + + " \"expires-at-ms\" : 1000\n" + + " } ]\n" + + "}", + metadata.lastUpdatedMillis()); + + String json = LoadTableResponseParser.toJson(response, true); + assertThat(json).isEqualTo(expectedJson); + // can't do an equality comparison because Schema doesn't implement equals/hashCode + assertThat(LoadTableResponseParser.toJson(LoadTableResponseParser.fromJson(json), true)) + .isEqualTo(expectedJson); + } + + @Test + public void unknownCredentials() { + String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b"; + TableMetadata metadata = + TableMetadata.buildFromEmpty() + .assignUUID(uuid) + .setLocation("location") + .setCurrentSchema( + new Schema(Types.NestedField.required(1, "x", Types.LongType.get())), 1) + .addPartitionSpec(PartitionSpec.unpartitioned()) + .addSortOrder(SortOrder.unsorted()) + .discardChanges() + .withMetadataLocation("metadata-location") + .build(); + + LoadTableResponse expected = + LoadTableResponse.builder() + .withTableMetadata(metadata) + .addAllConfig(ImmutableMap.of("key1", "val1", "key2", "val2")) + .addCredential( + ImmutableAdlsCredential.builder() + .sasToken("sasToken") + .expiresAt(Instant.ofEpochMilli(1000)) + .build()) + .build(); + + String json = + String.format( + "{\n" + + " \"metadata-location\" : \"metadata-location\",\n" + + " \"metadata\" : {\n" + + " \"format-version\" : 2,\n" + + " \"table-uuid\" : \"386b9f01-002b-4d8c-b77f-42c3fd3b7c9b\",\n" + + " \"location\" : \"location\",\n" + + " \"last-sequence-number\" : 0,\n" + + " \"last-updated-ms\" : %s,\n" + + " \"last-column-id\" : 1,\n" + + " \"current-schema-id\" : 0,\n" + + " \"schemas\" : [ {\n" + + " \"type\" : \"struct\",\n" + + " \"schema-id\" : 0,\n" + + " \"fields\" : [ {\n" + + " \"id\" : 1,\n" + + " \"name\" : \"x\",\n" + + " \"required\" : true,\n" + + " \"type\" : \"long\"\n" + + " } ]\n" + + " } ],\n" + + " \"default-spec-id\" : 0,\n" + + " \"partition-specs\" : [ {\n" + + " \"spec-id\" : 0,\n" + + " \"fields\" : [ ]\n" + + " } ],\n" + + " \"last-partition-id\" : 999,\n" + + " \"default-sort-order-id\" : 0,\n" + + " \"sort-orders\" : [ {\n" + + " \"order-id\" : 0,\n" + + " \"fields\" : [ ]\n" + + " } ],\n" + + " \"properties\" : { },\n" + + " \"current-snapshot-id\" : -1,\n" + + " \"refs\" : { },\n" + + " \"snapshots\" : [ ],\n" + + " \"statistics\" : [ ],\n" + + " \"partition-statistics\" : [ ],\n" + + " \"snapshot-log\" : [ ],\n" + + " \"metadata-log\" : [ ]\n" + + " },\n" + + " \"config\" : {\n" + + " \"key1\" : \"val1\",\n" + + " \"key2\" : \"val2\"\n" + + " },\n" + + " \"storage-credentials\" : [ {\n" + + " \"type\" : \"unknown1\",\n" + + " \"scheme\" : \"gs\",\n" + + " \"token\" : \"gcsToken\",\n" + + " \"expires-at-ms\" : 1000\n" + + " }, {\n" + + " \"type\" : \"unknown2\",\n" + + " \"scheme\" : \"gs://custom-uri\",\n" + + " \"token\" : \"token\",\n" + + " \"expires-at-ms\" : 1000\n" + + " }, {\n" + + " \"type\" : \"adls\",\n" + + " \"scheme\" : \"afbs\",\n" + + " \"sas-token\" : \"sasToken\",\n" + + " \"expires-at-ms\" : 1000\n" + + " } ]\n" + + "}", + metadata.lastUpdatedMillis()); + + assertThat(LoadTableResponseParser.fromJson(json).credentials()) + .hasSize(1) + .isEqualTo(expected.credentials()); + } } diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java index f3de08cd2912..dd726fdf91e4 100644 --- a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java @@ -22,9 +22,12 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.fasterxml.jackson.databind.JsonNode; +import java.time.Instant; import org.apache.iceberg.Schema; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.rest.credentials.ImmutableAdlsCredential; +import org.apache.iceberg.rest.credentials.ImmutableGcsCredential; import org.apache.iceberg.types.Types; import org.apache.iceberg.view.ImmutableViewVersion; import org.apache.iceberg.view.ViewMetadata; @@ -245,4 +248,183 @@ public void roundTripSerdeWithConfig() { assertThat(LoadViewResponseParser.toJson(LoadViewResponseParser.fromJson(json), true)) .isEqualTo(expectedJson); } + + @Test + public void roundTripSerdeWithCredentials() { + String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b"; + ViewMetadata viewMetadata = + ViewMetadata.builder() + .assignUUID(uuid) + .setLocation("location") + .addSchema(new Schema(Types.NestedField.required(1, "x", Types.LongType.get()))) + .addVersion( + ImmutableViewVersion.builder() + .schemaId(0) + .versionId(1) + .timestampMillis(23L) + .defaultNamespace(Namespace.of("ns1")) + .build()) + .setCurrentVersionId(1) + .build(); + + LoadViewResponse response = + ImmutableLoadViewResponse.builder() + .metadata(viewMetadata) + .metadataLocation("custom-location") + .addCredentials( + ImmutableGcsCredential.builder() + .token("gcsToken") + .expiresAt(Instant.ofEpochMilli(1000)) + .build()) + .addCredentials( + ImmutableGcsCredential.builder() + .scheme("gs://custom-uri") + .token("token") + .expiresAt(Instant.ofEpochMilli(1000)) + .build()) + .addCredentials( + ImmutableAdlsCredential.builder() + .sasToken("sasToken") + .expiresAt(Instant.ofEpochMilli(1000)) + .build()) + .build(); + + String expectedJson = + "{\n" + + " \"metadata-location\" : \"custom-location\",\n" + + " \"metadata\" : {\n" + + " \"view-uuid\" : \"386b9f01-002b-4d8c-b77f-42c3fd3b7c9b\",\n" + + " \"format-version\" : 1,\n" + + " \"location\" : \"location\",\n" + + " \"schemas\" : [ {\n" + + " \"type\" : \"struct\",\n" + + " \"schema-id\" : 0,\n" + + " \"fields\" : [ {\n" + + " \"id\" : 1,\n" + + " \"name\" : \"x\",\n" + + " \"required\" : true,\n" + + " \"type\" : \"long\"\n" + + " } ]\n" + + " } ],\n" + + " \"current-version-id\" : 1,\n" + + " \"versions\" : [ {\n" + + " \"version-id\" : 1,\n" + + " \"timestamp-ms\" : 23,\n" + + " \"schema-id\" : 0,\n" + + " \"summary\" : { },\n" + + " \"default-namespace\" : [ \"ns1\" ],\n" + + " \"representations\" : [ ]\n" + + " } ],\n" + + " \"version-log\" : [ {\n" + + " \"timestamp-ms\" : 23,\n" + + " \"version-id\" : 1\n" + + " } ]\n" + + " },\n" + + " \"storage-credentials\" : [ {\n" + + " \"type\" : \"gcs\",\n" + + " \"scheme\" : \"gs\",\n" + + " \"token\" : \"gcsToken\",\n" + + " \"expires-at-ms\" : 1000\n" + + " }, {\n" + + " \"type\" : \"gcs\",\n" + + " \"scheme\" : \"gs://custom-uri\",\n" + + " \"token\" : \"token\",\n" + + " \"expires-at-ms\" : 1000\n" + + " }, {\n" + + " \"type\" : \"adls\",\n" + + " \"scheme\" : \"afbs\",\n" + + " \"sas-token\" : \"sasToken\",\n" + + " \"expires-at-ms\" : 1000\n" + + " } ]\n" + + "}"; + + String json = LoadViewResponseParser.toJson(response, true); + assertThat(json).isEqualTo(expectedJson); + // can't do an equality comparison because Schema doesn't implement equals/hashCode + assertThat(LoadViewResponseParser.toJson(LoadViewResponseParser.fromJson(json), true)) + .isEqualTo(expectedJson); + } + + @Test + public void unknownCredentials() { + String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b"; + ViewMetadata viewMetadata = + ViewMetadata.builder() + .assignUUID(uuid) + .setLocation("location") + .addSchema(new Schema(Types.NestedField.required(1, "x", Types.LongType.get()))) + .addVersion( + ImmutableViewVersion.builder() + .schemaId(0) + .versionId(1) + .timestampMillis(23L) + .defaultNamespace(Namespace.of("ns1")) + .build()) + .setCurrentVersionId(1) + .build(); + + LoadViewResponse expected = + ImmutableLoadViewResponse.builder() + .metadata(viewMetadata) + .metadataLocation("custom-location") + .addCredentials( + ImmutableAdlsCredential.builder() + .sasToken("sasToken") + .expiresAt(Instant.ofEpochMilli(1000)) + .build()) + .build(); + + String json = + "{\n" + + " \"metadata-location\" : \"custom-location\",\n" + + " \"metadata\" : {\n" + + " \"view-uuid\" : \"386b9f01-002b-4d8c-b77f-42c3fd3b7c9b\",\n" + + " \"format-version\" : 1,\n" + + " \"location\" : \"location\",\n" + + " \"schemas\" : [ {\n" + + " \"type\" : \"struct\",\n" + + " \"schema-id\" : 0,\n" + + " \"fields\" : [ {\n" + + " \"id\" : 1,\n" + + " \"name\" : \"x\",\n" + + " \"required\" : true,\n" + + " \"type\" : \"long\"\n" + + " } ]\n" + + " } ],\n" + + " \"current-version-id\" : 1,\n" + + " \"versions\" : [ {\n" + + " \"version-id\" : 1,\n" + + " \"timestamp-ms\" : 23,\n" + + " \"schema-id\" : 0,\n" + + " \"summary\" : { },\n" + + " \"default-namespace\" : [ \"ns1\" ],\n" + + " \"representations\" : [ ]\n" + + " } ],\n" + + " \"version-log\" : [ {\n" + + " \"timestamp-ms\" : 23,\n" + + " \"version-id\" : 1\n" + + " } ]\n" + + " },\n" + + " \"storage-credentials\" : [ {\n" + + " \"type\" : \"unknown1\",\n" + + " \"scheme\" : \"gs\",\n" + + " \"token\" : \"gcsToken\",\n" + + " \"expires-at-ms\" : 1000\n" + + " }, {\n" + + " \"type\" : \"unknown2\",\n" + + " \"scheme\" : \"gs://custom-uri\",\n" + + " \"token\" : \"token\",\n" + + " \"expires-at-ms\" : 1000\n" + + " }, {\n" + + " \"type\" : \"adls\",\n" + + " \"scheme\" : \"afbs\",\n" + + " \"sas-token\" : \"sasToken\",\n" + + " \"expires-at-ms\" : 1000\n" + + " } ]\n" + + "}"; + + assertThat(LoadViewResponseParser.fromJson(json).credentials()) + .hasSize(1) + .isEqualTo(expected.credentials()); + } } From 1b12c856f010ad72cf547a5aa3be458c4b606fdc Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Wed, 2 Oct 2024 09:39:18 +0200 Subject: [PATCH 2/4] Make prefix optional --- .../rest/credentials/AdlsCredential.java | 7 +- .../rest/credentials/CredentialParser.java | 22 +++--- .../rest/credentials/GcsCredential.java | 7 +- .../rest/credentials/S3Credential.java | 7 +- .../credentials/TestCredentialParser.java | 68 ++++++------------- .../TestLoadTableResponseParser.java | 8 +-- .../responses/TestLoadViewResponseParser.java | 8 +-- 7 files changed, 50 insertions(+), 77 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java index df3c51a67d77..c1d9cf870639 100644 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java @@ -19,14 +19,13 @@ package org.apache.iceberg.rest.credentials; import java.time.Instant; +import javax.annotation.Nullable; import org.immutables.value.Value; @Value.Immutable public interface AdlsCredential extends Credential { - @Value.Default - default String scheme() { - return "afbs"; - } + @Nullable + String prefix(); String sasToken(); diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java b/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java index c9ed8085db0f..4b47b542ee69 100644 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java @@ -36,7 +36,7 @@ public class CredentialParser { private static final String ADLS_TOKEN = "sas-token"; private static final String S3_TOKEN = "session-token"; private static final String EXPIRES_AT = "expires-at-ms"; - private static final String SCHEME = "scheme"; + private static final String PREFIX = "prefix"; private CredentialParser() {} @@ -56,23 +56,29 @@ public static void toJson(Credential credentials, JsonGenerator gen) throws IOEx if (credentials instanceof S3Credential) { S3Credential s3 = (S3Credential) credentials; gen.writeStringField(TYPE, S3); - gen.writeStringField(SCHEME, s3.scheme()); gen.writeStringField(ACCESS_KEY_ID, s3.accessKeyId()); gen.writeStringField(SECRET_ACCESS_KEY, s3.secretAccessKey()); gen.writeStringField(S3_TOKEN, s3.sessionToken()); gen.writeNumberField(EXPIRES_AT, s3.expiresAt().toEpochMilli()); + if (null != s3.prefix()) { + gen.writeStringField(PREFIX, s3.prefix()); + } } else if (credentials instanceof GcsCredential) { GcsCredential gcs = (GcsCredential) credentials; gen.writeStringField(TYPE, GCS); - gen.writeStringField(SCHEME, gcs.scheme()); gen.writeStringField(GCS_TOKEN, gcs.token()); gen.writeNumberField(EXPIRES_AT, gcs.expiresAt().toEpochMilli()); + if (null != gcs.prefix()) { + gen.writeStringField(PREFIX, gcs.prefix()); + } } else if (credentials instanceof AdlsCredential) { AdlsCredential adls = (AdlsCredential) credentials; gen.writeStringField(TYPE, ADLS); - gen.writeStringField(SCHEME, adls.scheme()); gen.writeStringField(ADLS_TOKEN, adls.sasToken()); gen.writeNumberField(EXPIRES_AT, adls.expiresAt().toEpochMilli()); + if (null != adls.prefix()) { + gen.writeStringField(PREFIX, adls.prefix()); + } } gen.writeEndObject(); @@ -85,11 +91,11 @@ public static Credential fromJson(String json) { public static Credential fromJson(JsonNode json) { Preconditions.checkArgument(null != json, "Cannot parse credential from null object"); String credentialType = JsonUtil.getString(TYPE, json); - String scheme = JsonUtil.getString(SCHEME, json); + String prefix = JsonUtil.getStringOrNull(PREFIX, json); switch (credentialType) { case S3: return ImmutableS3Credential.builder() - .scheme(scheme) + .prefix(prefix) .accessKeyId(JsonUtil.getString(ACCESS_KEY_ID, json)) .secretAccessKey(JsonUtil.getString(SECRET_ACCESS_KEY, json)) .sessionToken(JsonUtil.getString(S3_TOKEN, json)) @@ -97,13 +103,13 @@ public static Credential fromJson(JsonNode json) { .build(); case GCS: return ImmutableGcsCredential.builder() - .scheme(scheme) + .prefix(prefix) .token(JsonUtil.getString(GCS_TOKEN, json)) .expiresAt(Instant.ofEpochMilli(JsonUtil.getLong(EXPIRES_AT, json))) .build(); case ADLS: return ImmutableAdlsCredential.builder() - .scheme(scheme) + .prefix(prefix) .sasToken(JsonUtil.getString(ADLS_TOKEN, json)) .expiresAt(Instant.ofEpochMilli(JsonUtil.getLong(EXPIRES_AT, json))) .build(); diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java index d2005f284a25..c43a7f27273d 100644 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java @@ -19,14 +19,13 @@ package org.apache.iceberg.rest.credentials; import java.time.Instant; +import javax.annotation.Nullable; import org.immutables.value.Value; @Value.Immutable public interface GcsCredential extends Credential { - @Value.Default - default String scheme() { - return "gs"; - } + @Nullable + String prefix(); String token(); diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java index f1ac0fd1a20a..179c5ce8b3b9 100644 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java @@ -19,14 +19,13 @@ package org.apache.iceberg.rest.credentials; import java.time.Instant; +import javax.annotation.Nullable; import org.immutables.value.Value; @Value.Immutable public interface S3Credential extends Credential { - @Value.Default - default String scheme() { - return "s3"; - } + @Nullable + String prefix(); String accessKeyId(); diff --git a/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java b/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java index e9af460464d0..f132f96d34df 100644 --- a/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java @@ -57,7 +57,7 @@ public void s3Credential() { Credential credential = ImmutableS3Credential.builder() .accessKeyId("keyId") - .scheme("s3://custom-uri") + .prefix("s3://custom-uri") .secretAccessKey("accessKey") .sessionToken("sessionToken") .expiresAt(expiresAt) @@ -67,11 +67,11 @@ public void s3Credential() { String.format( "{\n" + " \"type\" : \"s3\",\n" - + " \"scheme\" : \"s3://custom-uri\",\n" + " \"access-key-id\" : \"keyId\",\n" + " \"secret-access-key\" : \"accessKey\",\n" + " \"session-token\" : \"sessionToken\",\n" - + " \"expires-at-ms\" : %s\n" + + " \"expires-at-ms\" : %s,\n" + + " \"prefix\" : \"s3://custom-uri\"\n" + "}", expiresAt.toEpochMilli()); @@ -93,19 +93,6 @@ public void s3CredentialWithMissingProperties() { + " \"session-token\" : \"sessionToken\"" + "}")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: scheme"); - - assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\n" - + " \"type\" : \"s3\",\n" - + " \"scheme\" : \"s3\",\n" - + " \"access-key-id\" : \"keyId\",\n" - + " \"secret-access-key\" : \"accessKey\",\n" - + " \"session-token\" : \"sessionToken\"" - + "}")) - .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing long: expires-at-ms"); assertThatThrownBy( @@ -113,7 +100,6 @@ public void s3CredentialWithMissingProperties() { CredentialParser.fromJson( "{\n" + " \"type\" : \"s3\",\n" - + " \"scheme\" : \"s3\",\n" + " \"access-key-id\" : \"keyId\",\n" + " \"secret-access-key\" : \"accessKey\",\n" + " \"expires-at-ms\" : \"1\"" @@ -126,7 +112,6 @@ public void s3CredentialWithMissingProperties() { CredentialParser.fromJson( "{\n" + " \"type\" : \"s3\",\n" - + " \"scheme\" : \"s3\",\n" + " \"access-key-id\" : \"keyId\",\n" + " \"session-token\" : \"sessionToken\",\n" + " \"expires-at-ms\" : \"1\"" @@ -139,7 +124,6 @@ public void s3CredentialWithMissingProperties() { CredentialParser.fromJson( "{\n" + " \"type\" : \"s3\",\n" - + " \"scheme\" : \"s3\",\n" + " \"secret-access-key\" : \"accessKey\",\n" + " \"expires-at-ms\" : \"1\"" + "}")) @@ -152,15 +136,19 @@ public void gcsCredential() { Instant expiresAt = Instant.now(); Credential credential = - ImmutableGcsCredential.builder().token("gcsToken").expiresAt(expiresAt).build(); + ImmutableGcsCredential.builder() + .token("gcsToken") + .expiresAt(expiresAt) + .prefix("gs://custom-uri") + .build(); String expectedJson = String.format( "{\n" + " \"type\" : \"gcs\",\n" - + " \"scheme\" : \"gs\",\n" + " \"token\" : \"gcsToken\",\n" - + " \"expires-at-ms\" : %s\n" + + " \"expires-at-ms\" : %s,\n" + + " \"prefix\" : \"gs://custom-uri\"\n" + "}", expiresAt.toEpochMilli()); @@ -174,21 +162,12 @@ public void gcsCredential() { public void gcsCredentialWithMissingProperties() { assertThatThrownBy(() -> CredentialParser.fromJson("{\"type\": \"gcs\"}")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: scheme"); + .hasMessage("Cannot parse missing string: token"); assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\"type\": \"gcs\",\"scheme\":\"gs\",\"token\": \"gcsToken\"}")) + () -> CredentialParser.fromJson("{\"type\": \"gcs\",\"token\": \"gcsToken\"}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing long: expires-at-ms"); - - assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\"type\": \"gcs\",\"scheme\":\"gs\",\"expires-at-ms\": 1}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: token"); } @Test @@ -196,15 +175,19 @@ public void adlsCredential() { Instant expiresAt = Instant.now(); Credential credential = - ImmutableAdlsCredential.builder().sasToken("sasToken").expiresAt(expiresAt).build(); + ImmutableAdlsCredential.builder() + .sasToken("sasToken") + .expiresAt(expiresAt) + .prefix("abfs://custom-uri") + .build(); String expectedJson = String.format( "{\n" + " \"type\" : \"adls\",\n" - + " \"scheme\" : \"afbs\",\n" + " \"sas-token\" : \"sasToken\",\n" - + " \"expires-at-ms\" : %s\n" + + " \"expires-at-ms\" : %s,\n" + + " \"prefix\" : \"abfs://custom-uri\"\n" + "}", expiresAt.toEpochMilli()); @@ -218,19 +201,10 @@ public void adlsCredential() { public void adlsCredentialWithMissingProperties() { assertThatThrownBy(() -> CredentialParser.fromJson("{\"type\": \"adls\"}")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: scheme"); + .hasMessage("Cannot parse missing string: sas-token"); assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\"type\": \"adls\",\"scheme\": \"afbs\",\"sas-token\": \"adlsToken\"}")) + () -> CredentialParser.fromJson("{\"type\": \"adls\",\"sas-token\": \"adlsToken\"}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing long: expires-at-ms"); - - assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\"type\": \"adls\",\"scheme\": \"afbs\",\"expires-at-ms\": 1}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: sas-token"); } } diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java index 4326d19e2496..da9d5e785335 100644 --- a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java @@ -230,7 +230,7 @@ public void roundTripSerdeWithCredentials() { .build()) .addCredential( ImmutableGcsCredential.builder() - .scheme("gs://custom-uri") + .prefix("gs://custom-uri") .token("token") .expiresAt(Instant.ofEpochMilli(1000)) .build()) @@ -289,17 +289,15 @@ public void roundTripSerdeWithCredentials() { + " },\n" + " \"storage-credentials\" : [ {\n" + " \"type\" : \"gcs\",\n" - + " \"scheme\" : \"gs\",\n" + " \"token\" : \"gcsToken\",\n" + " \"expires-at-ms\" : 1000\n" + " }, {\n" + " \"type\" : \"gcs\",\n" - + " \"scheme\" : \"gs://custom-uri\",\n" + " \"token\" : \"token\",\n" - + " \"expires-at-ms\" : 1000\n" + + " \"expires-at-ms\" : 1000,\n" + + " \"prefix\" : \"gs://custom-uri\"\n" + " }, {\n" + " \"type\" : \"adls\",\n" - + " \"scheme\" : \"afbs\",\n" + " \"sas-token\" : \"sasToken\",\n" + " \"expires-at-ms\" : 1000\n" + " } ]\n" diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java index dd726fdf91e4..ad6c32f30b38 100644 --- a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java @@ -278,7 +278,7 @@ public void roundTripSerdeWithCredentials() { .build()) .addCredentials( ImmutableGcsCredential.builder() - .scheme("gs://custom-uri") + .prefix("gs://custom-uri") .token("token") .expiresAt(Instant.ofEpochMilli(1000)) .build()) @@ -322,17 +322,15 @@ public void roundTripSerdeWithCredentials() { + " },\n" + " \"storage-credentials\" : [ {\n" + " \"type\" : \"gcs\",\n" - + " \"scheme\" : \"gs\",\n" + " \"token\" : \"gcsToken\",\n" + " \"expires-at-ms\" : 1000\n" + " }, {\n" + " \"type\" : \"gcs\",\n" - + " \"scheme\" : \"gs://custom-uri\",\n" + " \"token\" : \"token\",\n" - + " \"expires-at-ms\" : 1000\n" + + " \"expires-at-ms\" : 1000,\n" + + " \"prefix\" : \"gs://custom-uri\"\n" + " }, {\n" + " \"type\" : \"adls\",\n" - + " \"scheme\" : \"afbs\",\n" + " \"sas-token\" : \"sasToken\",\n" + " \"expires-at-ms\" : 1000\n" + " } ]\n" From 9f33b09699a9eee987ab77bc1caaee45f93c81e6 Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Thu, 10 Oct 2024 09:20:46 +0200 Subject: [PATCH 3/4] Generalize how credentials are represented --- .../rest/credentials/AdlsCredential.java | 33 ---- .../iceberg/rest/credentials/Credential.java | 10 +- .../rest/credentials/CredentialParser.java | 84 ++------- .../rest/credentials/GcsCredential.java | 33 ---- .../rest/credentials/S3Credential.java | 37 ---- .../responses/LoadTableResponseParser.java | 8 +- .../responses/LoadViewResponseParser.java | 8 +- .../credentials/TestCredentialParser.java | 170 +++++------------- .../TestLoadTableResponseParser.java | 154 ++++------------ .../responses/TestLoadViewResponseParser.java | 141 ++++----------- 10 files changed, 147 insertions(+), 531 deletions(-) delete mode 100644 core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java delete mode 100644 core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java delete mode 100644 core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java deleted file mode 100644 index c1d9cf870639..000000000000 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/AdlsCredential.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.iceberg.rest.credentials; - -import java.time.Instant; -import javax.annotation.Nullable; -import org.immutables.value.Value; - -@Value.Immutable -public interface AdlsCredential extends Credential { - @Nullable - String prefix(); - - String sasToken(); - - Instant expiresAt(); -} diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java index 74fa3218c209..46f4a37e562b 100644 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java @@ -18,4 +18,12 @@ */ package org.apache.iceberg.rest.credentials; -public interface Credential {} +import java.util.Map; +import org.immutables.value.Value; + +@Value.Immutable +public interface Credential { + String prefix(); + + Map config(); +} diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java b/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java index 4b47b542ee69..14314d6e4fb2 100644 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/CredentialParser.java @@ -21,65 +21,31 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; -import java.time.Instant; +import java.util.Map; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.util.JsonUtil; public class CredentialParser { - private static final String TYPE = "type"; - private static final String S3 = "s3"; - private static final String GCS = "gcs"; - private static final String ADLS = "adls"; - private static final String ACCESS_KEY_ID = "access-key-id"; - private static final String SECRET_ACCESS_KEY = "secret-access-key"; - private static final String GCS_TOKEN = "token"; - private static final String ADLS_TOKEN = "sas-token"; - private static final String S3_TOKEN = "session-token"; - private static final String EXPIRES_AT = "expires-at-ms"; private static final String PREFIX = "prefix"; + private static final String CONFIG = "config"; private CredentialParser() {} - public static String toJson(Credential credentials) { - return toJson(credentials, false); + public static String toJson(Credential credential) { + return toJson(credential, false); } - public static String toJson(Credential credentials, boolean pretty) { - return JsonUtil.generate(gen -> toJson(credentials, gen), pretty); + public static String toJson(Credential credential, boolean pretty) { + return JsonUtil.generate(gen -> toJson(credential, gen), pretty); } - public static void toJson(Credential credentials, JsonGenerator gen) throws IOException { - Preconditions.checkArgument(null != credentials, "Invalid credential: null"); + public static void toJson(Credential credential, JsonGenerator gen) throws IOException { + Preconditions.checkArgument(null != credential, "Invalid credential: null"); gen.writeStartObject(); - if (credentials instanceof S3Credential) { - S3Credential s3 = (S3Credential) credentials; - gen.writeStringField(TYPE, S3); - gen.writeStringField(ACCESS_KEY_ID, s3.accessKeyId()); - gen.writeStringField(SECRET_ACCESS_KEY, s3.secretAccessKey()); - gen.writeStringField(S3_TOKEN, s3.sessionToken()); - gen.writeNumberField(EXPIRES_AT, s3.expiresAt().toEpochMilli()); - if (null != s3.prefix()) { - gen.writeStringField(PREFIX, s3.prefix()); - } - } else if (credentials instanceof GcsCredential) { - GcsCredential gcs = (GcsCredential) credentials; - gen.writeStringField(TYPE, GCS); - gen.writeStringField(GCS_TOKEN, gcs.token()); - gen.writeNumberField(EXPIRES_AT, gcs.expiresAt().toEpochMilli()); - if (null != gcs.prefix()) { - gen.writeStringField(PREFIX, gcs.prefix()); - } - } else if (credentials instanceof AdlsCredential) { - AdlsCredential adls = (AdlsCredential) credentials; - gen.writeStringField(TYPE, ADLS); - gen.writeStringField(ADLS_TOKEN, adls.sasToken()); - gen.writeNumberField(EXPIRES_AT, adls.expiresAt().toEpochMilli()); - if (null != adls.prefix()) { - gen.writeStringField(PREFIX, adls.prefix()); - } - } + gen.writeStringField(PREFIX, credential.prefix()); + JsonUtil.writeStringMap(CONFIG, credential.config(), gen); gen.writeEndObject(); } @@ -90,32 +56,8 @@ public static Credential fromJson(String json) { public static Credential fromJson(JsonNode json) { Preconditions.checkArgument(null != json, "Cannot parse credential from null object"); - String credentialType = JsonUtil.getString(TYPE, json); - String prefix = JsonUtil.getStringOrNull(PREFIX, json); - switch (credentialType) { - case S3: - return ImmutableS3Credential.builder() - .prefix(prefix) - .accessKeyId(JsonUtil.getString(ACCESS_KEY_ID, json)) - .secretAccessKey(JsonUtil.getString(SECRET_ACCESS_KEY, json)) - .sessionToken(JsonUtil.getString(S3_TOKEN, json)) - .expiresAt(Instant.ofEpochMilli(JsonUtil.getLong(EXPIRES_AT, json))) - .build(); - case GCS: - return ImmutableGcsCredential.builder() - .prefix(prefix) - .token(JsonUtil.getString(GCS_TOKEN, json)) - .expiresAt(Instant.ofEpochMilli(JsonUtil.getLong(EXPIRES_AT, json))) - .build(); - case ADLS: - return ImmutableAdlsCredential.builder() - .prefix(prefix) - .sasToken(JsonUtil.getString(ADLS_TOKEN, json)) - .expiresAt(Instant.ofEpochMilli(JsonUtil.getLong(EXPIRES_AT, json))) - .build(); - - default: - return null; - } + String prefix = JsonUtil.getString(PREFIX, json); + Map config = JsonUtil.getStringMap(CONFIG, json); + return ImmutableCredential.builder().prefix(prefix).config(config).build(); } } diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java deleted file mode 100644 index c43a7f27273d..000000000000 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/GcsCredential.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.iceberg.rest.credentials; - -import java.time.Instant; -import javax.annotation.Nullable; -import org.immutables.value.Value; - -@Value.Immutable -public interface GcsCredential extends Credential { - @Nullable - String prefix(); - - String token(); - - Instant expiresAt(); -} diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java deleted file mode 100644 index 179c5ce8b3b9..000000000000 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/S3Credential.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.iceberg.rest.credentials; - -import java.time.Instant; -import javax.annotation.Nullable; -import org.immutables.value.Value; - -@Value.Immutable -public interface S3Credential extends Credential { - @Nullable - String prefix(); - - String accessKeyId(); - - String secretAccessKey(); - - String sessionToken(); - - Instant expiresAt(); -} diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponseParser.java b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponseParser.java index 9a161801a877..875403d703ab 100644 --- a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponseParser.java +++ b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponseParser.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; -import java.util.Optional; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; @@ -103,10 +102,9 @@ public static LoadTableResponse fromJson(JsonNode json) { Preconditions.checkArgument( credentials.isArray(), "Cannot parse credentials from non-array: %s", credentials); - credentials.forEach( - cred -> - Optional.ofNullable(CredentialParser.fromJson(cred)) - .ifPresent(builder::addCredential)); + for (JsonNode credential : credentials) { + builder.addCredential(CredentialParser.fromJson(credential)); + } } return builder.build(); diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java b/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java index 447b85b2b6bb..61d8fce1dd51 100644 --- a/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java +++ b/core/src/main/java/org/apache/iceberg/rest/responses/LoadViewResponseParser.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; -import java.util.Optional; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.rest.credentials.Credential; import org.apache.iceberg.rest.credentials.CredentialParser; @@ -98,10 +97,9 @@ public static LoadViewResponse fromJson(JsonNode json) { Preconditions.checkArgument( credentials.isArray(), "Cannot parse credentials from non-array: %s", credentials); - credentials.forEach( - cred -> - Optional.ofNullable(CredentialParser.fromJson(cred)) - .ifPresent(builder::addCredentials)); + for (JsonNode credential : credentials) { + builder.addCredentials(CredentialParser.fromJson(credential)); + } } return builder.build(); diff --git a/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java b/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java index f132f96d34df..123b5bc99e8a 100644 --- a/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java @@ -22,7 +22,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.fasterxml.jackson.databind.JsonNode; -import java.time.Instant; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; public class TestCredentialParser { @@ -38,42 +38,40 @@ public void nullAndEmptyCheck() { } @Test - public void invalidTypeOrMissingType() { - assertThat(CredentialParser.fromJson("{\"type\": \"unknown\",\"scheme\": \"s3\"}")).isNull(); - + public void missingFields() { assertThatThrownBy(() -> CredentialParser.fromJson("{}")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: type"); + .hasMessage("Cannot parse missing string: prefix"); - assertThatThrownBy(() -> CredentialParser.fromJson("{\"x\": \"y\"}")) + assertThatThrownBy(() -> CredentialParser.fromJson("{\"prefix\": \"y\"}")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: type"); + .hasMessage("Cannot parse missing map: config"); } @Test public void s3Credential() { - Instant expiresAt = Instant.now(); - Credential credential = - ImmutableS3Credential.builder() - .accessKeyId("keyId") + ImmutableCredential.builder() .prefix("s3://custom-uri") - .secretAccessKey("accessKey") - .sessionToken("sessionToken") - .expiresAt(expiresAt) + .config( + ImmutableMap.of( + "s3.access-key-id", + "keyId", + "s3.secret-access-key", + "accessKey", + "s3.session-token", + "sessionToken")) .build(); String expectedJson = - String.format( - "{\n" - + " \"type\" : \"s3\",\n" - + " \"access-key-id\" : \"keyId\",\n" - + " \"secret-access-key\" : \"accessKey\",\n" - + " \"session-token\" : \"sessionToken\",\n" - + " \"expires-at-ms\" : %s,\n" - + " \"prefix\" : \"s3://custom-uri\"\n" - + "}", - expiresAt.toEpochMilli()); + "{\n" + + " \"prefix\" : \"s3://custom-uri\",\n" + + " \"config\" : {\n" + + " \"s3.access-key-id\" : \"keyId\",\n" + + " \"s3.secret-access-key\" : \"accessKey\",\n" + + " \"s3.session-token\" : \"sessionToken\"\n" + + " }\n" + + "}"; String json = CredentialParser.toJson(credential, true); assertThat(json).isEqualTo(expectedJson); @@ -81,76 +79,24 @@ public void s3Credential() { .isEqualTo(expectedJson); } - @Test - public void s3CredentialWithMissingProperties() { - assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\n" - + " \"type\" : \"s3\",\n" - + " \"access-key-id\" : \"keyId\",\n" - + " \"secret-access-key\" : \"accessKey\",\n" - + " \"session-token\" : \"sessionToken\"" - + "}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing long: expires-at-ms"); - - assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\n" - + " \"type\" : \"s3\",\n" - + " \"access-key-id\" : \"keyId\",\n" - + " \"secret-access-key\" : \"accessKey\",\n" - + " \"expires-at-ms\" : \"1\"" - + "}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: session-token"); - - assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\n" - + " \"type\" : \"s3\",\n" - + " \"access-key-id\" : \"keyId\",\n" - + " \"session-token\" : \"sessionToken\",\n" - + " \"expires-at-ms\" : \"1\"" - + "}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: secret-access-key"); - - assertThatThrownBy( - () -> - CredentialParser.fromJson( - "{\n" - + " \"type\" : \"s3\",\n" - + " \"secret-access-key\" : \"accessKey\",\n" - + " \"expires-at-ms\" : \"1\"" - + "}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: access-key-id"); - } - @Test public void gcsCredential() { - Instant expiresAt = Instant.now(); - Credential credential = - ImmutableGcsCredential.builder() - .token("gcsToken") - .expiresAt(expiresAt) + ImmutableCredential.builder() .prefix("gs://custom-uri") + .config( + ImmutableMap.of( + "gcs.oauth2.token", "gcsToken", "gcs.oauth2.token-expires-at", "1000")) .build(); String expectedJson = - String.format( - "{\n" - + " \"type\" : \"gcs\",\n" - + " \"token\" : \"gcsToken\",\n" - + " \"expires-at-ms\" : %s,\n" - + " \"prefix\" : \"gs://custom-uri\"\n" - + "}", - expiresAt.toEpochMilli()); + "{\n" + + " \"prefix\" : \"gs://custom-uri\",\n" + + " \"config\" : {\n" + + " \"gcs.oauth2.token\" : \"gcsToken\",\n" + + " \"gcs.oauth2.token-expires-at\" : \"1000\"\n" + + " }\n" + + "}"; String json = CredentialParser.toJson(credential, true); assertThat(json).isEqualTo(expectedJson); @@ -158,53 +104,31 @@ public void gcsCredential() { .isEqualTo(expectedJson); } - @Test - public void gcsCredentialWithMissingProperties() { - assertThatThrownBy(() -> CredentialParser.fromJson("{\"type\": \"gcs\"}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: token"); - - assertThatThrownBy( - () -> CredentialParser.fromJson("{\"type\": \"gcs\",\"token\": \"gcsToken\"}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing long: expires-at-ms"); - } - @Test public void adlsCredential() { - Instant expiresAt = Instant.now(); - Credential credential = - ImmutableAdlsCredential.builder() - .sasToken("sasToken") - .expiresAt(expiresAt) + ImmutableCredential.builder() .prefix("abfs://custom-uri") + .config( + ImmutableMap.of( + "adls.sas-token.account", + "sasToken", + "adls.auth.shared-key.account.key", + "accountKey")) .build(); String expectedJson = - String.format( - "{\n" - + " \"type\" : \"adls\",\n" - + " \"sas-token\" : \"sasToken\",\n" - + " \"expires-at-ms\" : %s,\n" - + " \"prefix\" : \"abfs://custom-uri\"\n" - + "}", - expiresAt.toEpochMilli()); + "{\n" + + " \"prefix\" : \"abfs://custom-uri\",\n" + + " \"config\" : {\n" + + " \"adls.sas-token.account\" : \"sasToken\",\n" + + " \"adls.auth.shared-key.account.key\" : \"accountKey\"\n" + + " }\n" + + "}"; String json = CredentialParser.toJson(credential, true); assertThat(json).isEqualTo(expectedJson); assertThat(CredentialParser.toJson(CredentialParser.fromJson(json), true)) .isEqualTo(expectedJson); } - - @Test - public void adlsCredentialWithMissingProperties() { - assertThatThrownBy(() -> CredentialParser.fromJson("{\"type\": \"adls\"}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing string: sas-token"); - assertThatThrownBy( - () -> CredentialParser.fromJson("{\"type\": \"adls\",\"sas-token\": \"adlsToken\"}")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse missing long: expires-at-ms"); - } } diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java index da9d5e785335..cc6f4cfc74d7 100644 --- a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponseParser.java @@ -22,14 +22,12 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.fasterxml.jackson.databind.JsonNode; -import java.time.Instant; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Schema; import org.apache.iceberg.SortOrder; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; -import org.apache.iceberg.rest.credentials.ImmutableAdlsCredential; -import org.apache.iceberg.rest.credentials.ImmutableGcsCredential; +import org.apache.iceberg.rest.credentials.ImmutableCredential; import org.apache.iceberg.types.Types; import org.junit.jupiter.api.Test; @@ -224,20 +222,30 @@ public void roundTripSerdeWithCredentials() { .withTableMetadata(metadata) .addAllConfig(ImmutableMap.of("key1", "val1", "key2", "val2")) .addCredential( - ImmutableGcsCredential.builder() - .token("gcsToken") - .expiresAt(Instant.ofEpochMilli(1000)) + ImmutableCredential.builder() + .prefix("s3://custom-uri") + .config( + ImmutableMap.of( + "s3.access-key-id", + "keyId", + "s3.secret-access-key", + "accessKey", + "s3.session-token", + "sessionToken")) .build()) .addCredential( - ImmutableGcsCredential.builder() + ImmutableCredential.builder() .prefix("gs://custom-uri") - .token("token") - .expiresAt(Instant.ofEpochMilli(1000)) + .config( + ImmutableMap.of( + "gcs.oauth2.token", "gcsToken1", "gcs.oauth2.token-expires-at", "1000")) .build()) .addCredential( - ImmutableAdlsCredential.builder() - .sasToken("sasToken") - .expiresAt(Instant.ofEpochMilli(1000)) + ImmutableCredential.builder() + .prefix("gs") + .config( + ImmutableMap.of( + "gcs.oauth2.token", "gcsToken2", "gcs.oauth2.token-expires-at", "2000")) .build()) .build(); @@ -288,18 +296,24 @@ public void roundTripSerdeWithCredentials() { + " \"key2\" : \"val2\"\n" + " },\n" + " \"storage-credentials\" : [ {\n" - + " \"type\" : \"gcs\",\n" - + " \"token\" : \"gcsToken\",\n" - + " \"expires-at-ms\" : 1000\n" + + " \"prefix\" : \"s3://custom-uri\",\n" + + " \"config\" : {\n" + + " \"s3.access-key-id\" : \"keyId\",\n" + + " \"s3.secret-access-key\" : \"accessKey\",\n" + + " \"s3.session-token\" : \"sessionToken\"\n" + + " }\n" + " }, {\n" - + " \"type\" : \"gcs\",\n" - + " \"token\" : \"token\",\n" - + " \"expires-at-ms\" : 1000,\n" - + " \"prefix\" : \"gs://custom-uri\"\n" + + " \"prefix\" : \"gs://custom-uri\",\n" + + " \"config\" : {\n" + + " \"gcs.oauth2.token\" : \"gcsToken1\",\n" + + " \"gcs.oauth2.token-expires-at\" : \"1000\"\n" + + " }\n" + " }, {\n" - + " \"type\" : \"adls\",\n" - + " \"sas-token\" : \"sasToken\",\n" - + " \"expires-at-ms\" : 1000\n" + + " \"prefix\" : \"gs\",\n" + + " \"config\" : {\n" + + " \"gcs.oauth2.token\" : \"gcsToken2\",\n" + + " \"gcs.oauth2.token-expires-at\" : \"2000\"\n" + + " }\n" + " } ]\n" + "}", metadata.lastUpdatedMillis()); @@ -310,100 +324,4 @@ public void roundTripSerdeWithCredentials() { assertThat(LoadTableResponseParser.toJson(LoadTableResponseParser.fromJson(json), true)) .isEqualTo(expectedJson); } - - @Test - public void unknownCredentials() { - String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b"; - TableMetadata metadata = - TableMetadata.buildFromEmpty() - .assignUUID(uuid) - .setLocation("location") - .setCurrentSchema( - new Schema(Types.NestedField.required(1, "x", Types.LongType.get())), 1) - .addPartitionSpec(PartitionSpec.unpartitioned()) - .addSortOrder(SortOrder.unsorted()) - .discardChanges() - .withMetadataLocation("metadata-location") - .build(); - - LoadTableResponse expected = - LoadTableResponse.builder() - .withTableMetadata(metadata) - .addAllConfig(ImmutableMap.of("key1", "val1", "key2", "val2")) - .addCredential( - ImmutableAdlsCredential.builder() - .sasToken("sasToken") - .expiresAt(Instant.ofEpochMilli(1000)) - .build()) - .build(); - - String json = - String.format( - "{\n" - + " \"metadata-location\" : \"metadata-location\",\n" - + " \"metadata\" : {\n" - + " \"format-version\" : 2,\n" - + " \"table-uuid\" : \"386b9f01-002b-4d8c-b77f-42c3fd3b7c9b\",\n" - + " \"location\" : \"location\",\n" - + " \"last-sequence-number\" : 0,\n" - + " \"last-updated-ms\" : %s,\n" - + " \"last-column-id\" : 1,\n" - + " \"current-schema-id\" : 0,\n" - + " \"schemas\" : [ {\n" - + " \"type\" : \"struct\",\n" - + " \"schema-id\" : 0,\n" - + " \"fields\" : [ {\n" - + " \"id\" : 1,\n" - + " \"name\" : \"x\",\n" - + " \"required\" : true,\n" - + " \"type\" : \"long\"\n" - + " } ]\n" - + " } ],\n" - + " \"default-spec-id\" : 0,\n" - + " \"partition-specs\" : [ {\n" - + " \"spec-id\" : 0,\n" - + " \"fields\" : [ ]\n" - + " } ],\n" - + " \"last-partition-id\" : 999,\n" - + " \"default-sort-order-id\" : 0,\n" - + " \"sort-orders\" : [ {\n" - + " \"order-id\" : 0,\n" - + " \"fields\" : [ ]\n" - + " } ],\n" - + " \"properties\" : { },\n" - + " \"current-snapshot-id\" : -1,\n" - + " \"refs\" : { },\n" - + " \"snapshots\" : [ ],\n" - + " \"statistics\" : [ ],\n" - + " \"partition-statistics\" : [ ],\n" - + " \"snapshot-log\" : [ ],\n" - + " \"metadata-log\" : [ ]\n" - + " },\n" - + " \"config\" : {\n" - + " \"key1\" : \"val1\",\n" - + " \"key2\" : \"val2\"\n" - + " },\n" - + " \"storage-credentials\" : [ {\n" - + " \"type\" : \"unknown1\",\n" - + " \"scheme\" : \"gs\",\n" - + " \"token\" : \"gcsToken\",\n" - + " \"expires-at-ms\" : 1000\n" - + " }, {\n" - + " \"type\" : \"unknown2\",\n" - + " \"scheme\" : \"gs://custom-uri\",\n" - + " \"token\" : \"token\",\n" - + " \"expires-at-ms\" : 1000\n" - + " }, {\n" - + " \"type\" : \"adls\",\n" - + " \"scheme\" : \"afbs\",\n" - + " \"sas-token\" : \"sasToken\",\n" - + " \"expires-at-ms\" : 1000\n" - + " } ]\n" - + "}", - metadata.lastUpdatedMillis()); - - assertThat(LoadTableResponseParser.fromJson(json).credentials()) - .hasSize(1) - .isEqualTo(expected.credentials()); - } } diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java index ad6c32f30b38..086db0fec8b4 100644 --- a/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestLoadViewResponseParser.java @@ -22,12 +22,10 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.fasterxml.jackson.databind.JsonNode; -import java.time.Instant; import org.apache.iceberg.Schema; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; -import org.apache.iceberg.rest.credentials.ImmutableAdlsCredential; -import org.apache.iceberg.rest.credentials.ImmutableGcsCredential; +import org.apache.iceberg.rest.credentials.ImmutableCredential; import org.apache.iceberg.types.Types; import org.apache.iceberg.view.ImmutableViewVersion; import org.apache.iceberg.view.ViewMetadata; @@ -272,20 +270,30 @@ public void roundTripSerdeWithCredentials() { .metadata(viewMetadata) .metadataLocation("custom-location") .addCredentials( - ImmutableGcsCredential.builder() - .token("gcsToken") - .expiresAt(Instant.ofEpochMilli(1000)) + ImmutableCredential.builder() + .prefix("s3://custom-uri") + .config( + ImmutableMap.of( + "s3.access-key-id", + "keyId", + "s3.secret-access-key", + "accessKey", + "s3.session-token", + "sessionToken")) .build()) .addCredentials( - ImmutableGcsCredential.builder() + ImmutableCredential.builder() .prefix("gs://custom-uri") - .token("token") - .expiresAt(Instant.ofEpochMilli(1000)) + .config( + ImmutableMap.of( + "gcs.oauth2.token", "gcsToken1", "gcs.oauth2.token-expires-at", "1000")) .build()) .addCredentials( - ImmutableAdlsCredential.builder() - .sasToken("sasToken") - .expiresAt(Instant.ofEpochMilli(1000)) + ImmutableCredential.builder() + .prefix("gs") + .config( + ImmutableMap.of( + "gcs.oauth2.token", "gcsToken2", "gcs.oauth2.token-expires-at", "2000")) .build()) .build(); @@ -321,18 +329,24 @@ public void roundTripSerdeWithCredentials() { + " } ]\n" + " },\n" + " \"storage-credentials\" : [ {\n" - + " \"type\" : \"gcs\",\n" - + " \"token\" : \"gcsToken\",\n" - + " \"expires-at-ms\" : 1000\n" + + " \"prefix\" : \"s3://custom-uri\",\n" + + " \"config\" : {\n" + + " \"s3.access-key-id\" : \"keyId\",\n" + + " \"s3.secret-access-key\" : \"accessKey\",\n" + + " \"s3.session-token\" : \"sessionToken\"\n" + + " }\n" + " }, {\n" - + " \"type\" : \"gcs\",\n" - + " \"token\" : \"token\",\n" - + " \"expires-at-ms\" : 1000,\n" - + " \"prefix\" : \"gs://custom-uri\"\n" + + " \"prefix\" : \"gs://custom-uri\",\n" + + " \"config\" : {\n" + + " \"gcs.oauth2.token\" : \"gcsToken1\",\n" + + " \"gcs.oauth2.token-expires-at\" : \"1000\"\n" + + " }\n" + " }, {\n" - + " \"type\" : \"adls\",\n" - + " \"sas-token\" : \"sasToken\",\n" - + " \"expires-at-ms\" : 1000\n" + + " \"prefix\" : \"gs\",\n" + + " \"config\" : {\n" + + " \"gcs.oauth2.token\" : \"gcsToken2\",\n" + + " \"gcs.oauth2.token-expires-at\" : \"2000\"\n" + + " }\n" + " } ]\n" + "}"; @@ -342,87 +356,4 @@ public void roundTripSerdeWithCredentials() { assertThat(LoadViewResponseParser.toJson(LoadViewResponseParser.fromJson(json), true)) .isEqualTo(expectedJson); } - - @Test - public void unknownCredentials() { - String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b"; - ViewMetadata viewMetadata = - ViewMetadata.builder() - .assignUUID(uuid) - .setLocation("location") - .addSchema(new Schema(Types.NestedField.required(1, "x", Types.LongType.get()))) - .addVersion( - ImmutableViewVersion.builder() - .schemaId(0) - .versionId(1) - .timestampMillis(23L) - .defaultNamespace(Namespace.of("ns1")) - .build()) - .setCurrentVersionId(1) - .build(); - - LoadViewResponse expected = - ImmutableLoadViewResponse.builder() - .metadata(viewMetadata) - .metadataLocation("custom-location") - .addCredentials( - ImmutableAdlsCredential.builder() - .sasToken("sasToken") - .expiresAt(Instant.ofEpochMilli(1000)) - .build()) - .build(); - - String json = - "{\n" - + " \"metadata-location\" : \"custom-location\",\n" - + " \"metadata\" : {\n" - + " \"view-uuid\" : \"386b9f01-002b-4d8c-b77f-42c3fd3b7c9b\",\n" - + " \"format-version\" : 1,\n" - + " \"location\" : \"location\",\n" - + " \"schemas\" : [ {\n" - + " \"type\" : \"struct\",\n" - + " \"schema-id\" : 0,\n" - + " \"fields\" : [ {\n" - + " \"id\" : 1,\n" - + " \"name\" : \"x\",\n" - + " \"required\" : true,\n" - + " \"type\" : \"long\"\n" - + " } ]\n" - + " } ],\n" - + " \"current-version-id\" : 1,\n" - + " \"versions\" : [ {\n" - + " \"version-id\" : 1,\n" - + " \"timestamp-ms\" : 23,\n" - + " \"schema-id\" : 0,\n" - + " \"summary\" : { },\n" - + " \"default-namespace\" : [ \"ns1\" ],\n" - + " \"representations\" : [ ]\n" - + " } ],\n" - + " \"version-log\" : [ {\n" - + " \"timestamp-ms\" : 23,\n" - + " \"version-id\" : 1\n" - + " } ]\n" - + " },\n" - + " \"storage-credentials\" : [ {\n" - + " \"type\" : \"unknown1\",\n" - + " \"scheme\" : \"gs\",\n" - + " \"token\" : \"gcsToken\",\n" - + " \"expires-at-ms\" : 1000\n" - + " }, {\n" - + " \"type\" : \"unknown2\",\n" - + " \"scheme\" : \"gs://custom-uri\",\n" - + " \"token\" : \"token\",\n" - + " \"expires-at-ms\" : 1000\n" - + " }, {\n" - + " \"type\" : \"adls\",\n" - + " \"scheme\" : \"afbs\",\n" - + " \"sas-token\" : \"sasToken\",\n" - + " \"expires-at-ms\" : 1000\n" - + " } ]\n" - + "}"; - - assertThat(LoadViewResponseParser.fromJson(json).credentials()) - .hasSize(1) - .isEqualTo(expected.credentials()); - } } From e38a18f0c1101237f94376abfec104f98da03c05 Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Thu, 17 Oct 2024 16:40:57 +0200 Subject: [PATCH 4/4] updates --- .../apache/iceberg/rest/credentials/Credential.java | 7 +++++++ .../iceberg/rest/responses/LoadTableResponse.java | 1 - .../rest/credentials/TestCredentialParser.java | 11 ++++++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java b/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java index 46f4a37e562b..0bd6673384de 100644 --- a/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java +++ b/core/src/main/java/org/apache/iceberg/rest/credentials/Credential.java @@ -19,6 +19,7 @@ package org.apache.iceberg.rest.credentials; import java.util.Map; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.immutables.value.Value; @Value.Immutable @@ -26,4 +27,10 @@ public interface Credential { String prefix(); Map config(); + + @Value.Check + default void validate() { + Preconditions.checkArgument(!prefix().isEmpty(), "Invalid prefix: must be non-empty"); + Preconditions.checkArgument(!config().isEmpty(), "Invalid config: must be non-empty"); + } } diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java index d52cdbbc253a..977220e7d782 100644 --- a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java +++ b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java @@ -93,7 +93,6 @@ public String toString() { .add("metadataLocation", metadataLocation) .add("metadata", metadata) .add("config", config) - .add("credentials", credentials) .toString(); } diff --git a/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java b/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java index 123b5bc99e8a..a48fd7353b98 100644 --- a/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/credentials/TestCredentialParser.java @@ -38,7 +38,7 @@ public void nullAndEmptyCheck() { } @Test - public void missingFields() { + public void invalidOrMissingFields() { assertThatThrownBy(() -> CredentialParser.fromJson("{}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing string: prefix"); @@ -46,6 +46,15 @@ public void missingFields() { assertThatThrownBy(() -> CredentialParser.fromJson("{\"prefix\": \"y\"}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing map: config"); + + assertThatThrownBy( + () -> CredentialParser.fromJson("{\"prefix\": \"\", \"config\": {\"x\": \"23\"}}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid prefix: must be non-empty"); + + assertThatThrownBy(() -> CredentialParser.fromJson("{\"prefix\": \"s3\", \"config\": {}}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid config: must be non-empty"); } @Test