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

HDDS-11149. Have a generic version Validator for validating Requests #6932

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Merge remote-tracking branch 'apache/master' into HEAD
Change-Id: Ic084d8bc456468d66cb26d8fc0cfdd33920fb8ed

# Conflicts:
#	hadoop-hdds/common/src/main/resources/META-INF/services/javax.annotation.processing.Processor
  • Loading branch information
swamirishi committed Jul 15, 2024
commit 363d9fa8ca102c8eb769d317f70b68af84bcfaf1
12 changes: 12 additions & 0 deletions hadoop-hdds/annotations/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,16 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<properties>
<maven.test.skip>true</maven.test.skip> <!-- no tests in this module so far -->
</properties>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<proc>none</proc>
</configuration>
</plugin>
</plugins>
</build>
</project>
37 changes: 37 additions & 0 deletions hadoop-hdds/client/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,43 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<excludeFilterFile>${basedir}/dev-support/findbugsExcludeFile.xml</excludeFilterFile>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths>
<path>
<groupId>org.apache.ozone</groupId>
<artifactId>hdds-config</artifactId>
<version>${hdds.version}</version>
</path>
</annotationProcessorPaths>
<annotationProcessors>
<annotationProcessor>org.apache.hadoop.hdds.conf.ConfigFileGenerator</annotationProcessor>
</annotationProcessors>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<executions>
<execution>
<id>ban-annotations</id> <!-- override default restriction from root POM -->
<configuration>
<rules>
<restrictImports>
<reason>Only selected annotation processors are enabled, see configuration of maven-compiler-plugin.</reason>
<bannedImports>
<bannedImport>org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator</bannedImport>
<bannedImport>org.apache.hadoop.hdds.scm.metadata.Replicate</bannedImport>
<bannedImport>org.kohsuke.MetaInfServices</bannedImport>
</bannedImports>
</restrictImports>
</rules>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ public ContainerCommandResponseProto sendCommand(
}
for (DatanodeDetails dn : datanodeList) {
try {
request = reconstructRequestIfNeeded(request, dn);
futureHashMap.put(dn, sendCommandAsync(request, dn).getResponse());
} catch (InterruptedException e) {
LOG.error("Command execution was interrupted.");
Expand Down Expand Up @@ -316,6 +317,29 @@ public ContainerCommandResponseProto sendCommand(
return responseProtoHashMap;
}

/**
* @param request
* @param dn
* @param pipeline
* In case of getBlock for EC keys, it is required to set replicaIndex for
* every request with the replicaIndex for that DN for which the request is
* sent to. This method unpacks proto and reconstructs request after setting
* the replicaIndex field.
* @return new updated Request
*/
private ContainerCommandRequestProto reconstructRequestIfNeeded(
ContainerCommandRequestProto request, DatanodeDetails dn) {
boolean isEcRequest = pipeline.getReplicationConfig()
.getReplicationType() == HddsProtos.ReplicationType.EC;
if (request.hasGetBlock() && isEcRequest) {
ContainerProtos.GetBlockRequestProto gbr = request.getGetBlock();
request = request.toBuilder().setGetBlock(gbr.toBuilder().setBlockID(
gbr.getBlockID().toBuilder().setReplicaIndex(
pipeline.getReplicaIndex(dn)).build()).build()).build();
}
return request;
}

@Override
public ContainerCommandResponseProto sendCommand(
ContainerCommandRequestProto request, List<Validator> validators)
Expand Down
37 changes: 37 additions & 0 deletions hadoop-hdds/common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,43 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<excludeFilterFile>${basedir}/dev-support/findbugsExcludeFile.xml</excludeFilterFile>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths>
<path>
<groupId>org.apache.ozone</groupId>
<artifactId>hdds-config</artifactId>
<version>${hdds.version}</version>
</path>
</annotationProcessorPaths>
<annotationProcessors>
<annotationProcessor>org.apache.hadoop.hdds.conf.ConfigFileGenerator</annotationProcessor>
</annotationProcessors>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<executions>
<execution>
<id>ban-annotations</id> <!-- override default restriction from root POM -->
<configuration>
<rules>
<restrictImports>
<reason>Only selected annotation processors are enabled, see configuration of maven-compiler-plugin.</reason>
<bannedImports>
<bannedImport>org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator</bannedImport>
<bannedImport>org.apache.hadoop.hdds.scm.metadata.Replicate</bannedImport>
<bannedImport>org.kohsuke.MetaInfServices</bannedImport>
</bannedImports>
</restrictImports>
</rules>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/*
* 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
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* 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.hadoop.hdds.conf;

import org.apache.hadoop.ozone.OzoneConfigKeys;
import org.apache.hadoop.util.StringUtils;

import java.util.Collection;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Properties;
import java.util.Set;

import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_CRYPTO_COMPLIANCE_MODE_UNRESTRICTED;

/**
* Delegating properties helper class. It's needed for configuration related classes, so we are able
* to delegate the operations that are happening on their Properties object to their parent's
* Properties object. This is needed because of the configuration compliance checks.
*/
public class DelegatingProperties extends Properties {
private final Properties properties;
private final String complianceMode;
private final boolean checkCompliance;
private final Properties cryptoProperties;

public DelegatingProperties(Properties properties, String complianceMode, Properties cryptoProperties) {
this.properties = properties;
this.complianceMode = complianceMode;
this.checkCompliance = !complianceMode.equals(OZONE_SECURITY_CRYPTO_COMPLIANCE_MODE_UNRESTRICTED);
this.cryptoProperties = cryptoProperties;
}

public String checkCompliance(String config, String value) {
// Don't check the ozone.security.crypto.compliance.mode config, even though it's tagged as a crypto config
if (checkCompliance && cryptoProperties.containsKey(config) &&
!config.equals(OzoneConfigKeys.OZONE_SECURITY_CRYPTO_COMPLIANCE_MODE)) {

String whitelistConfig = config + "." + complianceMode + ".whitelist";
String whitelistValue = properties.getProperty(whitelistConfig, "");

if (whitelistValue != null) {
Collection<String> whitelistOptions = StringUtils.getTrimmedStringCollection(whitelistValue);

if (!whitelistOptions.contains(value)) {
throw new ConfigurationException("Not allowed configuration value! Compliance mode is set to " +
complianceMode + " and " + config + " configuration's value is not allowed. Please check the " +
whitelistConfig + " configuration.");
}
}
}
return value;
}

@Override
public String getProperty(String key) {
String value = properties.getProperty(key);
return checkCompliance(key, value);
}

@Override
public Object setProperty(String key, String value) {
return properties.setProperty(key, value);
}

@Override
public synchronized Object remove(Object key) {
return properties.remove(key);
}

@Override
public synchronized void clear() {
properties.clear();
}

@Override
public Enumeration<Object> keys() {
return properties.keys();
}

@Override
public Enumeration<?> propertyNames() {
return properties.propertyNames();
}

@Override
public Set<String> stringPropertyNames() {
return properties.stringPropertyNames();
}

@Override
public int size() {
return properties.size();
}

@Override
public boolean isEmpty() {
return properties.isEmpty();
}

@Override
public Set<Object> keySet() {
return properties.keySet();
}

@Override
public boolean contains(Object value) {
return properties.contains(value);
}

@Override
public boolean containsKey(Object key) {
return properties.containsKey(key);
}

@Override
public boolean containsValue(Object value) {
return properties.containsValue(value);
}

@Override
public Object get(Object key) {
return properties.get(key);
}

@Override
public synchronized boolean remove(Object key, Object value) {
return properties.remove(key, value);
}

@Override
public synchronized Object put(Object key, Object value) {
return properties.put(key, value);
}

@Override
public synchronized void putAll(Map<?, ?> t) {
properties.putAll(t);
}

@Override
public Collection<Object> values() {
return properties.values();
}

@Override
public Set<Map.Entry<Object, Object>> entrySet() {
return properties.entrySet();
}

@Override
public synchronized boolean equals(Object o) {
return properties.equals(o);
}

@Override
public synchronized int hashCode() {
return properties.hashCode();
}

public Iterator<Map.Entry<String, String>> iterator() {
Map<String, String> result = new HashMap<>();
synchronized (properties) {
for (Map.Entry<Object, Object> item : properties.entrySet()) {
if (item.getKey() instanceof String && item.getValue() instanceof String) {
String value = checkCompliance((String) item.getKey(), (String) item.getValue());
result.put((String) item.getKey(), value);
}
}
}
return result.entrySet().iterator();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Properties;
Expand Down Expand Up @@ -407,4 +408,52 @@ public int getInt(String name, String fallbackName, int defaultValue,
}
return Integer.parseInt(value);
}

private Properties delegatingProps;

@Override
public synchronized void reloadConfiguration() {
super.reloadConfiguration();
delegatingProps = null;
}

@Override
protected final synchronized Properties getProps() {
if (delegatingProps == null) {
String complianceMode = getPropertyUnsafe(OzoneConfigKeys.OZONE_SECURITY_CRYPTO_COMPLIANCE_MODE,
OzoneConfigKeys.OZONE_SECURITY_CRYPTO_COMPLIANCE_MODE_UNRESTRICTED);
Properties cryptoProperties = getCryptoProperties();
delegatingProps = new DelegatingProperties(super.getProps(), complianceMode, cryptoProperties);
}
return delegatingProps;
}

/**
* Get a property value without the compliance check. It's needed to get the compliance
* mode from the configuration.
*
* @param key property name
* @param defaultValue default value
* @return property value, without compliance check
*/
private String getPropertyUnsafe(String key, String defaultValue) {
return super.getProps().getProperty(key, defaultValue);
}

private Properties getCryptoProperties() {
try {
return super.getAllPropertiesByTag(ConfigTag.CRYPTO_COMPLIANCE.toString());
} catch (NoSuchMethodError e) {
// We need to handle NoSuchMethodError, because in Hadoop 2 we don't have the
// getAllPropertiesByTag method. We won't be supporting the compliance mode with
// that version, so we are safe to catch the exception and return a new Properties object.
return new Properties();
}
}

@Override
public Iterator<Map.Entry<String, String>> iterator() {
DelegatingProperties properties = (DelegatingProperties) getProps();
return properties.iterator();
}
}
Loading
You are viewing a condensed version of this merge commit. You can view the full changes here.