Prabal864 commented on PR #17540:
URL: https://github.com/apache/iceberg/pull/17540#issuecomment-5386076245
Hi @waterWang @uros-b - I opened a duplicate PR (#17752, now closed in favor
of this one) with the identical production fix. One thing mine added that this
PR doesn't have is a regression test for the actual failure mode - a custom
credentials provider that re-derives its own `AwsClientProperties` from the
handed-in properties map, which NPEs on the null
`VendedCredentialsProvider.URI` value. Sharing it here in case it's useful:
```java
public static class CapturingCredentialProvider implements
AwsCredentialsProvider {
private static Map<String, String> lastProperties;
public static AwsCredentialsProvider create(Map<String, String>
properties) {
lastProperties = properties;
// Mirrors a real-world custom provider that re-derives its own
AwsClientProperties
// from the properties it is handed; this is what surfaces a null value
as an NPE.
PropertyUtil.filterProperties(properties, key -> true);
return new CapturingCredentialProvider();
}
@Override
public AwsCredentials resolveCredentials() {
return AwsBasicCredentials.builder().build();
}
}
@Test
public void
customCredentialsProviderWithoutRefreshEndpointDoesNotPassNullUri() {
// No REFRESH_CREDENTIALS_ENDPOINT and no CatalogProperties.URI set, so
// refreshCredentialsEndpoint resolves to null. A custom provider must not
be handed a
// properties map containing a null value for
VendedCredentialsProvider.URI, since some
// implementations (e.g. ones that re-derive their own AwsClientProperties
from the map)
// will NPE on that null value.
AwsClientProperties awsClientProperties =
new AwsClientProperties(
ImmutableMap.of(
AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER,
CapturingCredentialProvider.class.getName()));
AwsCredentialsProvider provider =
awsClientProperties.credentialsProvider("key", "secret", "token");
assertThat(provider).isInstanceOf(CapturingCredentialProvider.class);
assertThat(CapturingCredentialProvider.lastProperties)
.as("properties handed to a custom provider should never contain a
null value")
.doesNotContainValue(null);
}
```
Feel free to take it, adapt it, or ignore it - just didn't want the coverage
to go to waste.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
