Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to OpenSearch 2.0.0 #1698

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Sets the version of the Security plugin
security-plugin.version=2.0.0.0
# Sets the version of OpenSearch this plugin should be built with
opensearch-core.version=1.4.0
opensearch-core.version=2.0.0
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public boolean doStore(final AuditMessage msg) {

try (StoredContext ctx = threadPool.getThreadContext().stashContext()) {
try {
final IndexRequestBuilder irb = clientProvider.prepareIndex(getExpandedIndexName(indexPattern, index), type).setRefreshPolicy(RefreshPolicy.IMMEDIATE).setSource(msg.getAsMap());
final IndexRequestBuilder irb = clientProvider.prepareIndex(getExpandedIndexName(indexPattern, index)).setRefreshPolicy(RefreshPolicy.IMMEDIATE).setSource(msg.getAsMap());
threadPool.getThreadContext().putHeader(ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true");
irb.setTimeout(TimeValue.timeValueMinutes(1));
irb.execute().actionGet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public Index preIndex(final ShardId shardId, final Index index) {
if (shard.isReadAllowed()) {
try {

final GetResult getResult = shard.getService().getForUpdate(index.type(), index.id(),
final GetResult getResult = shard.getService().getForUpdate(index.id(),
index.getIfSeqNo(), index.getIfPrimaryTerm());

if (getResult.isExists()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
public interface ConfigCallback {

void success(SecurityDynamicConfiguration<?> dConf);
void noData(String id, String type);
void noData(String id);
void singleFailure(Failure failure);
void failure(Throwable t);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,17 @@ public void singleFailure(Failure failure) {
}

@Override
public void noData(String id, String type) {
public void noData(String id) {
CType cType = CType.fromString(id);

//when index was created with ES 6 there are no separate tenants. So we load just empty ones.
//when index was created with ES 7 and type not "security" (ES 6 type) there are no rolemappings anymore.
if(cs.state().metadata().index(securityIndex).getCreationVersion().before(LegacyESVersion.V_7_0_0) || "security".equals(type)) {
if(cs.state().metadata().index(securityIndex).getCreationVersion().before(LegacyESVersion.V_7_0_0)) {
//created with SG 6
//skip tenants

if (isDebugEnabled) {
log.debug("Skip tenants because we not yet migrated to ES 7 (index was created with ES 6 and type is legacy [{}])", type);
log.debug("Skip tenants because we not yet migrated to ES 7 (index was created with ES 6)");
}

if(cType == CType.TENANTS) {
Expand Down Expand Up @@ -175,7 +175,7 @@ public void noData(String id, String type) {
}
}

log.warn("No data for {} while retrieving configuration for {} (index={} and type={})", id, Arrays.toString(events), securityIndex, type);
log.warn("No data for {} while retrieving configuration for {} (index={})", id, Arrays.toString(events), securityIndex);
}

@Override
Expand Down Expand Up @@ -231,7 +231,7 @@ public void onResponse(MultiGetResponse response) {
}
} else {
//does not exist or empty source
callback.noData(singleGetResponse.getId(), singleGetResponse.getType());
callback.noData(singleGetResponse.getId());
}
} else {
//failure
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public void onResponse(SearchResponse response) {
if (hits == 1) {
getListener.onResponse(new GetResponse(searchHitToGetResult(response.getHits().getAt(0))));
} else if (hits == 0) {
getListener.onResponse(new GetResponse(new GetResult(searchRequest.indices()[0], "_doc", getRequest.id(),
getListener.onResponse(new GetResponse(new GetResult(searchRequest.indices()[0], getRequest.id(),
SequenceNumbers.UNASSIGNED_SEQ_NO, SequenceNumbers.UNASSIGNED_PRIMARY_TERM, -1, false, null, null, null)));
} else {
log.error("Unexpected hit count " + hits + " in " + response);
Expand Down Expand Up @@ -386,10 +386,7 @@ private GetResult searchHitToGetResult(SearchHit hit) {
}
}

@SuppressWarnings("deprecation")
String type = hit.getType();

return new GetResult(hit.getIndex(), type, hit.getId(), hit.getSeqNo(), hit.getPrimaryTerm(), hit.getVersion(), true, hit.getSourceRef(),
return new GetResult(hit.getIndex(), hit.getId(), hit.getSeqNo(), hit.getPrimaryTerm(), hit.getVersion(), true, hit.getSourceRef(),
documentFields, metadataFields);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public void onResponse(AcknowledgedResponse response) {
public void onResponse(CreateIndexResponse response) {
final List<SecurityDynamicConfiguration<?>> dynamicConfigurations = builder.build();
final ImmutableList.Builder<String> cTypes = ImmutableList.builderWithExpectedSize(dynamicConfigurations.size());
final BulkRequestBuilder br = client.prepareBulk(opendistroIndex, "_doc");
final BulkRequestBuilder br = client.prepareBulk(opendistroIndex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, why is this change reqd?

br.setRefreshPolicy(RefreshPolicy.IMMEDIATE);
try {
for (SecurityDynamicConfiguration dynamicConfiguration : dynamicConfigurations) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public boolean index(final String content, final String index, final String type

try {

final IndexRequest ir = type==null?new IndexRequest(index):new IndexRequest(index, type);
final IndexRequest ir = new IndexRequest(index);

final IndexResponse response = rclient.index(ir
.setRefreshPolicy(refresh?RefreshPolicy.IMMEDIATE:RefreshPolicy.NONE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public final void messageReceived(T request, TransportChannel channel, Task task
protected TransportChannel getInnerChannel(TransportChannel transportChannel) throws Exception {
try {
Class wrappedChannelCls = transportChannel.getClass();
Method getInnerChannel = wrappedChannelCls.getMethod("getInnerChannel", null);
Method getInnerChannel = wrappedChannelCls.getMethod("getInnerChannel");
TransportChannel innerChannel = (TransportChannel)(getInnerChannel.invoke(transportChannel));
log.debug("Using inner transport channel " + innerChannel.getChannelType());
return innerChannel;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public static void uploadFile(Client tc, String filepath, String index, CType cT
try (Reader reader = createFileOrStringReader(cType, configVersion, filepath, populateEmptyIfFileMissing)) {

final IndexRequest indexRequest = new IndexRequest(index)
.type(configVersion == 1 ? "security" : "_doc")
.id(configType)
.opType(OpType.CREATE)
.setRefreshPolicy(RefreshPolicy.IMMEDIATE)
Expand Down
27 changes: 11 additions & 16 deletions src/main/java/org/opensearch/security/tools/SecurityAdmin.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,10 @@
import org.opensearch.action.admin.cluster.health.ClusterHealthRequest;
import org.opensearch.action.admin.cluster.health.ClusterHealthResponse;
import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.opensearch.action.admin.indices.create.CreateIndexRequest;
import org.opensearch.client.indices.CreateIndexRequest;
import org.opensearch.action.admin.indices.delete.DeleteIndexRequest;
import org.opensearch.action.admin.indices.get.GetIndexRequest;
import org.opensearch.action.admin.indices.get.GetIndexRequest.Feature;
import org.opensearch.action.admin.indices.get.GetIndexResponse;
import org.opensearch.client.indices.GetIndexRequest;
import org.opensearch.client.indices.GetIndexRequest.Feature;
import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest;
import org.opensearch.action.get.GetRequest;
import org.opensearch.action.get.GetResponse;
Expand All @@ -75,6 +74,7 @@
import org.opensearch.client.RestClient;
import org.opensearch.client.RestClientBuilder;
import org.opensearch.client.RestHighLevelClient;
import org.opensearch.client.indices.GetIndexResponse;
import org.opensearch.client.transport.NoNodeAvailableException;
import org.opensearch.cluster.health.ClusterHealthStatus;
import org.opensearch.common.Strings;
Expand Down Expand Up @@ -655,7 +655,7 @@ public static int execute(final String[] args) throws Exception {

GetIndexResponse securityIndex = null;
try {
securityIndex = restHighLevelClient.indices().get(new GetIndexRequest().indices(index).addFeatures(Feature.MAPPINGS), RequestOptions.DEFAULT);
securityIndex = restHighLevelClient.indices().get(new GetIndexRequest(index).addFeatures(Feature.MAPPINGS), RequestOptions.DEFAULT);
} catch (OpenSearchStatusException e1) {
if(e1.status() == RestStatus.NOT_FOUND) {
//ignore
Expand Down Expand Up @@ -715,8 +715,7 @@ public static int execute(final String[] args) throws Exception {

final boolean legacy = createLegacyMode || (indexExists
&& securityIndex.getMappings() != null
&& securityIndex.getMappings().get(index) != null
&& securityIndex.getMappings().get(index).containsKey("security"));
&& securityIndex.getMappings().get(index) != null);

if(legacy) {
System.out.println("Legacy index '"+index+"' (ES 6) detected (or forced). You should migrate the configuration!");
Expand Down Expand Up @@ -832,15 +831,13 @@ private static boolean uploadFile(final RestHighLevelClient restHighLevelClient,
private static boolean uploadFile(final RestHighLevelClient restHighLevelClient, final String filepath, final String index, final String _id, final boolean legacy, boolean resolveEnvVars,
final boolean populateEmptyIfMissing) {

String type = "_doc";
String id = _id;

if(legacy) {
type = "security";
id = _id;

try {
ConfigHelper.fromYamlFile(filepath, CType.fromString(_id), 1, 0, 0);
ConfigHelper.fromYamlFile(filepath, CType.fromString(_id), 2, 0, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean for this value to switch from 1 -> 2? Additional documentation would be useful in this code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was for migration Config 6 -> Config 7. But OpenSearch doesn't need to support this migration. Removing the 2 test classes for the same reason.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include documentation in the code as to how this value is determine or aligned to. This could also be done with a local variable eg. final int latestSupportedMajorVersion = 2;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea! Can we do that in a separate PR? Let's unblock 2.0.0 build first.

} catch (Exception e) {
System.out.println("ERR: Seems "+filepath+" is not in legacy format: "+e);
return false;
Expand All @@ -855,12 +852,12 @@ private static boolean uploadFile(final RestHighLevelClient restHighLevelClient,
}
}

System.out.println("Will update '" + type + "/" + id + "' with " + filepath + " " + (legacy ? "(legacy mode)" : ""));
System.out.println("Will update '" + "/" + id + "' with " + filepath + " " + (legacy ? "(legacy mode)" : ""));
cliu123 marked this conversation as resolved.
Show resolved Hide resolved

try (Reader reader = ConfigHelper.createFileOrStringReader(CType.fromString(_id), legacy ? 1 : 2, filepath, populateEmptyIfMissing)) {
final String content = CharStreams.toString(reader);
final String res = restHighLevelClient
.index(new IndexRequest(index).type(type).id(id).setRefreshPolicy(RefreshPolicy.IMMEDIATE)
.index(new IndexRequest(index).id(id).setRefreshPolicy(RefreshPolicy.IMMEDIATE)
.source(_id, readXContent(resolveEnvVars ? replaceEnvVars(content, Settings.EMPTY) : content, XContentType.YAML)), RequestOptions.DEFAULT).getId();


Expand All @@ -883,19 +880,17 @@ private static boolean retrieveFile(final RestHighLevelClient restHighLevelClien
}

private static boolean retrieveFile(final RestHighLevelClient restHighLevelClient, final String filepath, final String index, final String _id, final boolean legacy, final boolean populateFileIfEmpty) {
String type = "_doc";
String id = _id;

if(legacy) {
type = "security";
id = _id;

}

System.out.println("Will retrieve '"+type+"/" +id+"' into "+filepath+" "+(legacy?"(legacy mode)":""));
System.out.println("Will retrieve '"+"/" +id+"' into "+filepath+" "+(legacy?"(legacy mode)":""));
try (Writer writer = new FileWriter(filepath)) {

final GetResponse response = restHighLevelClient.get(new GetRequest(index).type(type).id(id).refresh(true).realtime(false), RequestOptions.DEFAULT);
final GetResponse response = restHighLevelClient.get(new GetRequest(index).id(id).refresh(true).realtime(false), RequestOptions.DEFAULT);

boolean isEmpty = !response.isExists() || response.isSourceEmpty();
String yaml;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public void testWithPassiveAuthDisabled() throws Exception {

RestHelper.HttpResponse res;
RestHelper rh = nonSslRestHelper();
res = rh.executeGetRequest("/_cluster/health", null);
res = rh.executeGetRequest("/_cluster/health");
Assert.assertEquals(res.getBody(), HttpStatus.SC_INTERNAL_SERVER_ERROR, res.getStatusCode());
}

Expand All @@ -210,7 +210,7 @@ public void testWithPassiveAuthDisabledDynamic() throws Exception {

RestHelper.HttpResponse res;
RestHelper rh = nonSslRestHelper();
res = rh.executeGetRequest("/_cluster/health", null);
res = rh.executeGetRequest("/_cluster/health");
Assert.assertEquals(res.getBody(), HttpStatus.SC_INTERNAL_SERVER_ERROR, res.getStatusCode());

}
Expand Down
20 changes: 10 additions & 10 deletions src/test/java/org/opensearch/security/AggregationTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,18 @@ public void testBasicAggregations() throws Exception {

try (Client tc = getClient()) {
tc.admin().indices().create(new CreateIndexRequest("copysf")).actionGet();
tc.index(new IndexRequest("vulcangov").type("kolinahr").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet").type("ships").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet_academy").type("students").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet_library").type("public").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("klingonempire").type("ships").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("public").type("legends").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("vulcangov").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet_academy").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("starfleet_library").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("klingonempire").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("public").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();

tc.index(new IndexRequest("spock").type("type01").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("kirk").type("type01").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("role01_role02").type("type01").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("spock").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("kirk").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("role01_role02").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();

tc.index(new IndexRequest("xyz").type("doc").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();
tc.index(new IndexRequest("xyz").setRefreshPolicy(RefreshPolicy.IMMEDIATE).source("{\"content\":1}", XContentType.JSON)).actionGet();


tc.admin().indices().aliases(new IndicesAliasesRequest().addAliasAction(AliasActions.add().indices("starfleet","starfleet_academy","starfleet_library").alias("sf"))).actionGet();
Expand Down
Loading