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.

Sign up for GitHub

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

Jump to bottom

Remove references to LegacyESVersion.V_7x constants #2151

Merged
merged 3 commits into from
Oct 12, 2022
Merged

Remove references to LegacyESVersion.V_7x constants #2151

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b24b13
Remove references to LegacyESVersion.V_7x constants
cwperks Oct 10, 2022
9e95d8d
Remove unused oldestNodeVersion
cwperks Oct 10, 2022
7e74266
Remove unused imports
cwperks Oct 11, 2022
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
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/security/action/configupdate/TransportConfigUpdateAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

import org.opensearch.action.FailedNodeException;
import org.opensearch.action.support.ActionFilters;
import org.opensearch.action.support.nodes.BaseNodeRequest;
import org.opensearch.action.support.nodes.TransportNodesAction;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.inject.Inject;
Expand All @@ -47,6 +46,7 @@
import org.opensearch.security.securityconf.DynamicConfigFactory;
import org.opensearch.security.securityconf.impl.CType;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.TransportRequest;
import org.opensearch.transport.TransportService;

public class TransportConfigUpdateAction
Expand All @@ -72,7 +72,7 @@ public TransportConfigUpdateAction(final Settings settings,
this.dynamicConfigFactory = dynamicConfigFactory;
}

public static class NodeConfigUpdateRequest extends BaseNodeRequest {
public static class NodeConfigUpdateRequest extends TransportRequest {
Copy link
Member

Choose a reason for hiding this comment

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

What would this change achieve and does it break anything?

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 class was removed in 3.0.0. I followed the docstring

/**
 * Base class for node transport requests
 *
 * @opensearch.internal
 *
 * @deprecated this class is deprecated and classes will extend TransportRequest directly
 */
// TODO: this class can be removed in main once 7.x is bumped to 7.4.0
@Deprecated
public abstract class BaseNodeRequest extends TransportRequest {

See: https://github.com/opensearch-project/OpenSearch/pull/4702/files#diff-a8affa0470501c9cb284bfbd66f00d493fc72bf05d12e4e2c89bfda6fe814371L47


ConfigUpdateRequest request;

Expand Down
49 changes: 0 additions & 49 deletions src/main/java/org/opensearch/security/configuration/ClusterInfoHolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,49 +26,23 @@

package org.opensearch.security.configuration;

import java.util.Iterator;
import java.util.List;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import org.opensearch.LegacyESVersion;
import org.opensearch.cluster.ClusterChangedEvent;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ClusterStateListener;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.node.DiscoveryNode;
import org.opensearch.cluster.node.DiscoveryNodes;
import org.opensearch.index.Index;

public class ClusterInfoHolder implements ClusterStateListener {

protected final Logger log = LogManager.getLogger(this.getClass());
private volatile Boolean has6xNodes = null;
private volatile Boolean has6xIndices = null;
private volatile DiscoveryNodes nodes = null;
private volatile Boolean isLocalNodeElectedClusterManager = null;
private volatile boolean initialized;

@Override
public void clusterChanged(ClusterChangedEvent event) {
final boolean isTraceEnabled = log.isTraceEnabled();
if(has6xNodes == null || event.nodesChanged()) {
has6xNodes = Boolean.valueOf(clusterHas6xNodes(event.state()));
if (isTraceEnabled) {
log.trace("has6xNodes: {}", has6xNodes);
}
}

final List<String> indicesCreated = event.indicesCreated();
final List<Index> indicesDeleted = event.indicesDeleted();
if(has6xIndices == null || !indicesCreated.isEmpty() || !indicesDeleted.isEmpty()) {
has6xIndices = Boolean.valueOf(clusterHas6xIndices(event.state()));
if (isTraceEnabled) {
log.trace("has6xIndices: {}", has6xIndices);
}
}

if(nodes == null || event.nodesChanged()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We only need to track when nodes change now not also when indices are created & deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

The purpose of this code is to log if there are 6.X nodes or indices in the cluster upon change (node was added/removed from the cluster or an index was created or deleted). If its not possible to run es 6.X nodes with os 3.X then this code will never be used.

nodes = event.state().nodes();
if (log.isDebugEnabled()) {
Expand All @@ -80,14 +54,6 @@ public void clusterChanged(ClusterChangedEvent event) {
isLocalNodeElectedClusterManager = event.localNodeClusterManager()?Boolean.TRUE:Boolean.FALSE;
}

public Boolean getHas6xNodes() {
return has6xNodes;
}

public Boolean getHas6xIndices() {
return has6xIndices;
}

public Boolean isLocalNodeElectedClusterManager() {
return isLocalNodeElectedClusterManager;
}
Expand All @@ -106,19 +72,4 @@ public Boolean hasNode(DiscoveryNode node) {

return nodes.nodeExists(node)?Boolean.TRUE:Boolean.FALSE;
}

private static boolean clusterHas6xNodes(ClusterState state) {
cliu123 marked this conversation as resolved.
Show resolved Hide resolved
return state.nodes().getMinNodeVersion().before(LegacyESVersion.V_7_0_0);
}

private static boolean clusterHas6xIndices(ClusterState state) {
final Iterator<IndexMetadata> indices = state.metadata().indices().valuesIt();
while (indices.hasNext()) {
final IndexMetadata indexMetadata = indices.next();
if (indexMetadata.getCreationVersion().before(LegacyESVersion.V_7_0_0)) {
return true;
}
}
return false;
}
}
17 changes: 0 additions & 17 deletions src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,23 +125,6 @@ public void singleFailure(Failure failure) {
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)) {
//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)");
}

if(cType == CType.TENANTS) {
rs.put(cType, SecurityDynamicConfiguration.empty());
latch.countDown();
return;
}
}

// Since NODESDN is newly introduced data-type applying for existing clusters as well, we make it backward compatible by returning valid empty
// SecurityDynamicConfiguration.
// Same idea for new setting WHITELIST/ALLOWLIST
Expand Down
7 changes: 0 additions & 7 deletions src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,6 @@ protected Endpoint getEndpoint() {
@Override
protected void handlePost(RestChannel channel, RestRequest request, Client client, final JsonNode content) throws IOException {

final Version oldestNodeVersion = cs.state().getNodes().getMinNodeVersion();

if(oldestNodeVersion.before(LegacyESVersion.V_7_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.

Instead of removing this, should we be replacing it with oldest that we currently support?

badRequestResponse(channel, "Can not migrate configuration because cluster is not fully migrated.");
return;
}

final SecurityDynamicConfiguration<?> loadedConfig = load(CType.CONFIG, true);

if (loadedConfig.getVersion() != 1) {
Expand Down