Skip to content

Commit 28c1e5e

Browse files
rjernstrjernst
committed
PR feedback
1 parent f711224 commit 28c1e5e

File tree

5 files changed

+143
-130
lines changed

5 files changed

+143
-130
lines changed

‎plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/AwsS3Service.java‎

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -40,134 +40,135 @@ public interface AwsS3Service extends LifecycleComponent {
4040
/**
4141
* cloud.aws.access_key: AWS Access key. Shared with discovery-ec2 plugin
4242
*/
43-
Setting<SecureString> LEGACY_KEY_SETTING = new Setting<>("cloud.aws.access_key", "", SecureString::new,
43+
Setting<SecureString> KEY_SETTING = new Setting<>("cloud.aws.access_key", "", SecureString::new,
4444
Property.NodeScope, Property.Filtered, Property.Deprecated, Property.Shared);
4545
/**
4646
* cloud.aws.secret_key: AWS Secret key. Shared with discovery-ec2 plugin
4747
*/
48-
Setting<SecureString> LEGACY_SECRET_SETTING = new Setting<>("cloud.aws.secret_key", "", SecureString::new,
48+
Setting<SecureString> SECRET_SETTING = new Setting<>("cloud.aws.secret_key", "", SecureString::new,
4949
Property.NodeScope, Property.Filtered, Property.Deprecated, Property.Shared);
5050
/**
5151
* cloud.aws.protocol: Protocol for AWS API: http or https. Defaults to https. Shared with discovery-ec2 plugin
5252
*/
53-
Setting<Protocol> LEGACY_PROTOCOL_SETTING = new Setting<>("cloud.aws.protocol", "https",
53+
Setting<Protocol> PROTOCOL_SETTING = new Setting<>("cloud.aws.protocol", "https",
5454
s -> Protocol.valueOf(s.toUpperCase(Locale.ROOT)), Property.NodeScope, Property.Deprecated, Property.Shared);
5555
/**
5656
* cloud.aws.proxy.host: In case of proxy, define its hostname/IP. Shared with discovery-ec2 plugin
5757
*/
58-
Setting<String> LEGACY_PROXY_HOST_SETTING = Setting.simpleString("cloud.aws.proxy.host",
58+
Setting<String> PROXY_HOST_SETTING = Setting.simpleString("cloud.aws.proxy.host",
5959
Property.NodeScope, Property.Deprecated, Property.Shared);
6060
/**
6161
* cloud.aws.proxy.port: In case of proxy, define its port. Defaults to 80. Shared with discovery-ec2 plugin
6262
*/
63-
Setting<Integer> LEGACY_PROXY_PORT_SETTING = Setting.intSetting("cloud.aws.proxy.port", 80, 0, 1<<16,
63+
Setting<Integer> PROXY_PORT_SETTING = Setting.intSetting("cloud.aws.proxy.port", 80, 0, 1<<16,
6464
Property.NodeScope, Property.Deprecated, Property.Shared);
6565
/**
6666
* cloud.aws.proxy.username: In case of proxy with auth, define the username. Shared with discovery-ec2 plugin
6767
*/
68-
Setting<SecureString> LEGACY_PROXY_USERNAME_SETTING = new Setting<>("cloud.aws.proxy.username", "", SecureString::new,
68+
Setting<SecureString> PROXY_USERNAME_SETTING = new Setting<>("cloud.aws.proxy.username", "", SecureString::new,
6969
Property.NodeScope, Property.Deprecated, Property.Shared);
7070
/**
7171
* cloud.aws.proxy.password: In case of proxy with auth, define the password. Shared with discovery-ec2 plugin
7272
*/
73-
Setting<SecureString> LEGACY_PROXY_PASSWORD_SETTING = new Setting<>("cloud.aws.proxy.password", "", SecureString::new,
73+
Setting<SecureString> PROXY_PASSWORD_SETTING = new Setting<>("cloud.aws.proxy.password", "", SecureString::new,
7474
Property.NodeScope, Property.Filtered, Property.Deprecated, Property.Shared);
7575
/**
7676
* cloud.aws.signer: If you are using an old AWS API version, you can define a Signer. Shared with discovery-ec2 plugin
7777
*/
78-
Setting<String> LEGACY_SIGNER_SETTING = Setting.simpleString("cloud.aws.signer",
78+
Setting<String> SIGNER_SETTING = Setting.simpleString("cloud.aws.signer",
7979
Property.NodeScope, Property.Deprecated, Property.Shared);
8080
/**
8181
* cloud.aws.region: Region. Shared with discovery-ec2 plugin
8282
*/
83-
Setting<String> LEGACY_REGION_SETTING = new Setting<>("cloud.aws.region", "", s -> s.toLowerCase(Locale.ROOT),
83+
Setting<String> REGION_SETTING = new Setting<>("cloud.aws.region", "", s -> s.toLowerCase(Locale.ROOT),
8484
Property.NodeScope, Property.Deprecated, Property.Shared);
8585
/**
8686
* cloud.aws.read_timeout: Socket read timeout. Shared with discovery-ec2 plugin
8787
*/
88-
Setting<TimeValue> LEGACY_READ_TIMEOUT = Setting.timeSetting("cloud.aws.read_timeout",
88+
Setting<TimeValue> READ_TIMEOUT = Setting.timeSetting("cloud.aws.read_timeout",
8989
TimeValue.timeValueMillis(ClientConfiguration.DEFAULT_SOCKET_TIMEOUT), Property.NodeScope, Property.Deprecated, Property.Shared);
9090

9191
/**
9292
* Defines specific s3 settings starting with cloud.aws.s3.
93+
* NOTE: These are legacy settings. Use the named client configs in {@link org.elasticsearch.repositories.s3.S3Repository}.
9394
*/
9495
interface CLOUD_S3 {
9596
/**
9697
* cloud.aws.s3.access_key: AWS Access key specific for S3 API calls. Defaults to cloud.aws.access_key.
97-
* @see AwsS3Service#LEGACY_KEY_SETTING
98+
* @see AwsS3Service#KEY_SETTING
9899
*/
99100
Setting<SecureString> KEY_SETTING =
100-
new Setting<>("cloud.aws.s3.access_key", AwsS3Service.LEGACY_KEY_SETTING, SecureString::new,
101+
new Setting<>("cloud.aws.s3.access_key", AwsS3Service.KEY_SETTING, SecureString::new,
101102
Property.NodeScope, Property.Filtered, Property.Deprecated);
102103
/**
103104
* cloud.aws.s3.secret_key: AWS Secret key specific for S3 API calls. Defaults to cloud.aws.secret_key.
104-
* @see AwsS3Service#LEGACY_SECRET_SETTING
105+
* @see AwsS3Service#SECRET_SETTING
105106
*/
106107
Setting<SecureString> SECRET_SETTING =
107-
new Setting<>("cloud.aws.s3.secret_key", AwsS3Service.LEGACY_SECRET_SETTING, SecureString::new,
108+
new Setting<>("cloud.aws.s3.secret_key", AwsS3Service.SECRET_SETTING, SecureString::new,
108109
Property.NodeScope, Property.Filtered, Property.Deprecated);
109110
/**
110111
* cloud.aws.s3.protocol: Protocol for AWS API specific for S3 API calls: http or https. Defaults to cloud.aws.protocol.
111-
* @see AwsS3Service#LEGACY_PROTOCOL_SETTING
112+
* @see AwsS3Service#PROTOCOL_SETTING
112113
*/
113114
Setting<Protocol> PROTOCOL_SETTING =
114-
new Setting<>("cloud.aws.s3.protocol", AwsS3Service.LEGACY_PROTOCOL_SETTING, s -> Protocol.valueOf(s.toUpperCase(Locale.ROOT)),
115+
new Setting<>("cloud.aws.s3.protocol", AwsS3Service.PROTOCOL_SETTING, s -> Protocol.valueOf(s.toUpperCase(Locale.ROOT)),
115116
Property.NodeScope, Property.Deprecated);
116117
/**
117118
* cloud.aws.s3.proxy.host: In case of proxy, define its hostname/IP specific for S3 API calls. Defaults to cloud.aws.proxy.host.
118-
* @see AwsS3Service#LEGACY_PROXY_HOST_SETTING
119+
* @see AwsS3Service#PROXY_HOST_SETTING
119120
*/
120121
Setting<String> PROXY_HOST_SETTING =
121-
new Setting<>("cloud.aws.s3.proxy.host", AwsS3Service.LEGACY_PROXY_HOST_SETTING, Function.identity(),
122+
new Setting<>("cloud.aws.s3.proxy.host", AwsS3Service.PROXY_HOST_SETTING, Function.identity(),
122123
Property.NodeScope, Property.Deprecated);
123124
/**
124125
* cloud.aws.s3.proxy.port: In case of proxy, define its port specific for S3 API calls. Defaults to cloud.aws.proxy.port.
125-
* @see AwsS3Service#LEGACY_PROXY_PORT_SETTING
126+
* @see AwsS3Service#PROXY_PORT_SETTING
126127
*/
127128
Setting<Integer> PROXY_PORT_SETTING =
128-
new Setting<>("cloud.aws.s3.proxy.port", AwsS3Service.LEGACY_PROXY_PORT_SETTING,
129+
new Setting<>("cloud.aws.s3.proxy.port", AwsS3Service.PROXY_PORT_SETTING,
129130
s -> Setting.parseInt(s, 0, 1<<16, "cloud.aws.s3.proxy.port"), Property.NodeScope, Property.Deprecated);
130131
/**
131132
* cloud.aws.s3.proxy.username: In case of proxy with auth, define the username specific for S3 API calls.
132133
* Defaults to cloud.aws.proxy.username.
133-
* @see AwsS3Service#LEGACY_PROXY_USERNAME_SETTING
134+
* @see AwsS3Service#PROXY_USERNAME_SETTING
134135
*/
135136
Setting<SecureString> PROXY_USERNAME_SETTING =
136-
new Setting<>("cloud.aws.s3.proxy.username", AwsS3Service.LEGACY_PROXY_USERNAME_SETTING, SecureString::new,
137+
new Setting<>("cloud.aws.s3.proxy.username", AwsS3Service.PROXY_USERNAME_SETTING, SecureString::new,
137138
Property.NodeScope, Property.Deprecated);
138139
/**
139140
* cloud.aws.s3.proxy.password: In case of proxy with auth, define the password specific for S3 API calls.
140141
* Defaults to cloud.aws.proxy.password.
141-
* @see AwsS3Service#LEGACY_PROXY_PASSWORD_SETTING
142+
* @see AwsS3Service#PROXY_PASSWORD_SETTING
142143
*/
143144
Setting<SecureString> PROXY_PASSWORD_SETTING =
144-
new Setting<>("cloud.aws.s3.proxy.password", AwsS3Service.LEGACY_PROXY_PASSWORD_SETTING, SecureString::new,
145+
new Setting<>("cloud.aws.s3.proxy.password", AwsS3Service.PROXY_PASSWORD_SETTING, SecureString::new,
145146
Property.NodeScope, Property.Filtered, Property.Deprecated);
146147
/**
147148
* cloud.aws.s3.signer: If you are using an old AWS API version, you can define a Signer. Specific for S3 API calls.
148149
* Defaults to cloud.aws.signer.
149-
* @see AwsS3Service#LEGACY_SIGNER_SETTING
150+
* @see AwsS3Service#SIGNER_SETTING
150151
*/
151152
Setting<String> SIGNER_SETTING =
152-
new Setting<>("cloud.aws.s3.signer", AwsS3Service.LEGACY_SIGNER_SETTING, Function.identity(),
153+
new Setting<>("cloud.aws.s3.signer", AwsS3Service.SIGNER_SETTING, Function.identity(),
153154
Property.NodeScope, Property.Deprecated);
154155
/**
155156
* cloud.aws.s3.region: Region specific for S3 API calls. Defaults to cloud.aws.region.
156-
* @see AwsS3Service#LEGACY_REGION_SETTING
157+
* @see AwsS3Service#REGION_SETTING
157158
*/
158159
Setting<String> REGION_SETTING =
159-
new Setting<>("cloud.aws.s3.region", AwsS3Service.LEGACY_REGION_SETTING, s -> s.toLowerCase(Locale.ROOT),
160+
new Setting<>("cloud.aws.s3.region", AwsS3Service.REGION_SETTING, s -> s.toLowerCase(Locale.ROOT),
160161
Property.NodeScope, Property.Deprecated);
161162
/**
162163
* cloud.aws.s3.endpoint: Endpoint. If not set, endpoint will be guessed based on region setting.
163164
*/
164165
Setting<String> ENDPOINT_SETTING = Setting.simpleString("cloud.aws.s3.endpoint", Property.NodeScope);
165166
/**
166167
* cloud.aws.s3.read_timeout: Socket read timeout. Defaults to cloud.aws.read_timeout
167-
* @see AwsS3Service#LEGACY_READ_TIMEOUT
168+
* @see AwsS3Service#READ_TIMEOUT
168169
*/
169170
Setting<TimeValue> READ_TIMEOUT =
170-
Setting.timeSetting("cloud.aws.s3.read_timeout", AwsS3Service.LEGACY_READ_TIMEOUT, Property.NodeScope, Property.Deprecated);
171+
Setting.timeSetting("cloud.aws.s3.read_timeout", AwsS3Service.READ_TIMEOUT, Property.NodeScope, Property.Deprecated);
171172
}
172173

173174
AmazonS3 client(Settings repositorySettings, Integer maxRetries, boolean useThrottleRetries, Boolean pathStyleAccess);

‎plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,18 @@
1919

2020
package org.elasticsearch.cloud.aws;
2121

22+
import java.util.HashMap;
23+
import java.util.Map;
24+
import java.util.function.Function;
25+
2226
import com.amazonaws.ClientConfiguration;
2327
import com.amazonaws.Protocol;
2428
import com.amazonaws.auth.AWSCredentials;
2529
import com.amazonaws.auth.AWSCredentialsProvider;
2630
import com.amazonaws.auth.BasicAWSCredentials;
27-
import com.amazonaws.auth.DefaultAWSCredentialsProviderChain;
2831
import com.amazonaws.auth.EnvironmentVariableCredentialsProvider;
2932
import com.amazonaws.auth.InstanceProfileCredentialsProvider;
3033
import com.amazonaws.auth.SystemPropertiesCredentialsProvider;
31-
import com.amazonaws.auth.profile.ProfileCredentialsProvider;
3234
import com.amazonaws.http.IdleConnectionReaper;
3335
import com.amazonaws.internal.StaticCredentialsProvider;
3436
import com.amazonaws.services.s3.AmazonS3;
@@ -46,17 +48,10 @@
4648
import org.elasticsearch.common.unit.TimeValue;
4749
import org.elasticsearch.repositories.s3.S3Repository;
4850

49-
import java.util.HashMap;
50-
import java.util.Map;
51-
import java.util.function.Function;
52-
import java.util.function.Supplier;
53-
54-
import static org.elasticsearch.repositories.s3.S3Repository.getValue;
55-
5651
public class InternalAwsS3Service extends AbstractLifecycleComponent implements AwsS3Service {
5752

5853
// pkg private for tests
59-
static final Setting<String> CONFIG_NAME = new Setting<>("config", "default", Function.identity());
54+
static final Setting<String> CLIENT_NAME = new Setting<>("client", "default", Function.identity());
6055

6156
/**
6257
* (acceskey, endpoint) -&gt; client
@@ -70,7 +65,7 @@ public InternalAwsS3Service(Settings settings) {
7065
@Override
7166
public synchronized AmazonS3 client(Settings repositorySettings, Integer maxRetries,
7267
boolean useThrottleRetries, Boolean pathStyleAccess) {
73-
String configName = CONFIG_NAME.get(repositorySettings);
68+
String configName = CLIENT_NAME.get(repositorySettings);
7469
String foundEndpoint = findEndpoint(logger, repositorySettings, settings, configName);
7570

7671
AWSCredentialsProvider credentials = buildCredentials(logger, deprecationLogger, settings, repositorySettings, configName);
@@ -213,7 +208,7 @@ static String findEndpoint(Logger logger, Settings repositorySettings, Settings
213208
if (CLOUD_S3.ENDPOINT_SETTING.exists(settings)) {
214209
endpoint = CLOUD_S3.ENDPOINT_SETTING.get(settings);
215210
logger.debug("using explicit s3 endpoint [{}]", endpoint);
216-
} else if (LEGACY_REGION_SETTING.exists(settings) || CLOUD_S3.REGION_SETTING.exists(settings)) {
211+
} else if (REGION_SETTING.exists(settings) || CLOUD_S3.REGION_SETTING.exists(settings)) {
217212
region = CLOUD_S3.REGION_SETTING.get(settings);
218213
endpoint = getEndpoint(region);
219214
logger.debug("using s3 region [{}], with endpoint [{}]", region, endpoint);
@@ -230,7 +225,7 @@ static String findEndpoint(Logger logger, Settings repositorySettings, Settings
230225
* Return the region configured, or empty string.
231226
* TODO: remove after https://github.com/elastic/elasticsearch/issues/22761 */
232227
public static String getRegion(Settings repositorySettings, Settings settings) {
233-
return getConfigValue(repositorySettings, settings, CONFIG_NAME.get(repositorySettings), S3Repository.REGION_SETTING,
228+
return getConfigValue(repositorySettings, settings, CLIENT_NAME.get(repositorySettings), S3Repository.REGION_SETTING,
234229
S3Repository.Repository.REGION_SETTING, S3Repository.Repositories.REGION_SETTING);
235230
}
236231

‎plugins/repository-s3/src/main/java/org/elasticsearch/plugin/repository/s3/S3RepositoryPlugin.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,16 +82,16 @@ public List<String> getSettingsFilter() {
8282
public List<Setting<?>> getSettings() {
8383
return Arrays.asList(
8484
// Register global cloud aws settings: cloud.aws (might have been registered in ec2 plugin)
85-
AwsS3Service.LEGACY_KEY_SETTING,
86-
AwsS3Service.LEGACY_SECRET_SETTING,
87-
AwsS3Service.LEGACY_PROTOCOL_SETTING,
88-
AwsS3Service.LEGACY_PROXY_HOST_SETTING,
89-
AwsS3Service.LEGACY_PROXY_PORT_SETTING,
90-
AwsS3Service.LEGACY_PROXY_USERNAME_SETTING,
91-
AwsS3Service.LEGACY_PROXY_PASSWORD_SETTING,
92-
AwsS3Service.LEGACY_SIGNER_SETTING,
93-
AwsS3Service.LEGACY_REGION_SETTING,
94-
AwsS3Service.LEGACY_READ_TIMEOUT,
85+
AwsS3Service.KEY_SETTING,
86+
AwsS3Service.SECRET_SETTING,
87+
AwsS3Service.PROTOCOL_SETTING,
88+
AwsS3Service.PROXY_HOST_SETTING,
89+
AwsS3Service.PROXY_PORT_SETTING,
90+
AwsS3Service.PROXY_USERNAME_SETTING,
91+
AwsS3Service.PROXY_PASSWORD_SETTING,
92+
AwsS3Service.SIGNER_SETTING,
93+
AwsS3Service.REGION_SETTING,
94+
AwsS3Service.READ_TIMEOUT,
9595

9696
// Register S3 specific settings: cloud.aws.s3
9797
AwsS3Service.CLOUD_S3.KEY_SETTING,

‎plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.amazonaws.ClientConfiguration;
2323
import com.amazonaws.Protocol;
2424
import com.amazonaws.services.s3.AmazonS3;
25-
import com.amazonaws.services.s3.AmazonS3Client;
2625
import org.elasticsearch.cloud.aws.AwsS3Service;
2726
import org.elasticsearch.cloud.aws.AwsS3Service.CLOUD_S3;
2827
import org.elasticsearch.cloud.aws.InternalAwsS3Service;
@@ -65,8 +64,8 @@ public class S3Repository extends BlobStoreRepository {
6564

6665
public static final String TYPE = "s3";
6766

68-
// prefix for aws client settings
69-
private static final String PREFIX = "aws.config.";
67+
// prefix for s3 client settings
68+
private static final String PREFIX = "s3.client.";
7069

7170
/** The access key (ie login id) for connecting to s3. */
7271
public static final AffixSetting<SecureString> ACCESS_KEY_SETTING = Setting.affixKeySetting(PREFIX, "access_key",
@@ -98,18 +97,19 @@ public class S3Repository extends BlobStoreRepository {
9897

9998
/** The username of a proxy to connect to s3 through. */
10099
public static final AffixSetting<SecureString> PROXY_USERNAME_SETTING = Setting.affixKeySetting(PREFIX, "proxy.username",
101-
key -> SecureSetting.secureString(key, AwsS3Service.LEGACY_PROXY_USERNAME_SETTING, false));
100+
key -> SecureSetting.secureString(key, AwsS3Service.PROXY_USERNAME_SETTING, false));
102101

103102
/** The password of a proxy to connect to s3 through. */
104103
public static final AffixSetting<SecureString> PROXY_PASSWORD_SETTING = Setting.affixKeySetting(PREFIX, "proxy.password",
105-
key -> SecureSetting.secureString(key, AwsS3Service.LEGACY_PROXY_PASSWORD_SETTING, false));
104+
key -> SecureSetting.secureString(key, AwsS3Service.PROXY_PASSWORD_SETTING, false));
106105

107106
/** The socket timeout for connecting to s3. */
108107
public static final AffixSetting<TimeValue> READ_TIMEOUT_SETTING = Setting.affixKeySetting(PREFIX, "read_timeout",
109108
key -> Setting.timeSetting(key, TimeValue.timeValueMillis(ClientConfiguration.DEFAULT_SOCKET_TIMEOUT), Property.NodeScope));
110109

111110
/**
112111
* Global S3 repositories settings. Starting with: repositories.s3
112+
* NOTE: These are legacy settings. Use the named client config settings above.
113113
*/
114114
public interface Repositories {
115115
/**

0 commit comments

Comments
 (0)