Skip to content

Commit

Permalink
fix FQDN machine name mapping on proxy configuration
Browse files Browse the repository at this point in the history
It was noticed that configuring a proxy sometimes would fail and just display a generic error.
Analysis can be split in two:
- what was causing the problem
Files uploaded during configuration are stored and moved based on a 'machine name' mapping based on the provided 'Proxy FQDN'.
https://github.com/uyuni-project/uyuni/blob/32465f6efccd319926ae1897e14d9d20d4d89c39/spacewalk/certs-tools/sslToolLib.py#L73
Current solution based on a slightly more generic occurrence:
 https://github.com/uyuni-project/uyuni/blob/f1f62028c363f8ea95b0cc080b4fa9eddf9fbaa0/spacewalk/certs-tools/rhn_ssl_tool.py#L205
- why was just a generic error
This had to do with our (rhn) custom FileUtils throwing generic RTEs while RhnRuntimeException is expected
https://github.com/uyuni-project/uyuni/blob/32465f6efccd319926ae1897e14d9d20d4d89c39/java/code/src/com/redhat/rhn/common/util/FileUtils.java#L130
  • Loading branch information
rjpmestre committed Oct 20, 2023
1 parent 32465f6 commit a4071bf
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 11 deletions.
6 changes: 4 additions & 2 deletions java/code/src/com/redhat/rhn/common/util/FileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
*/
package com.redhat.rhn.common.util;

import com.redhat.rhn.common.RhnRuntimeException;

import org.apache.commons.collections.buffer.CircularFifoBuffer;
import org.apache.commons.io.LineIterator;
import org.apache.logging.log4j.LogManager;
Expand Down Expand Up @@ -145,10 +147,10 @@ public static String readStringFromFile(String path, boolean noLog) {
return contents;
}
catch (FileNotFoundException e) {
throw new RuntimeException("File not found: " + path);
throw new RhnRuntimeException("File not found: " + path);
}
catch (IOException e) {
throw new RuntimeException(e);
throw new RhnRuntimeException(e);
}
}

Expand Down
15 changes: 7 additions & 8 deletions java/code/src/com/suse/manager/ssl/SSLCertData.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
*/
package com.suse.manager.ssl;

import org.apache.commons.lang3.StringUtils;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -139,12 +140,10 @@ public int hashCode() {
* @return machine name
*/
public String getMachineName() {
List<String> dataList = Arrays.asList(this.getCn().split("\\."));
if (dataList.size() > 2) {
return String.join(".", dataList.subList(0, dataList.size() - 2));
}
else {
return String.join(".");
}
String[] hostnameParts = this.getCn().split("\\.");

return hostnameParts.length > 2 ?
StringUtils.join(hostnameParts, ".", 0, hostnameParts.length - 2) :
this.getCn();
}
}
79 changes: 79 additions & 0 deletions java/code/src/com/suse/manager/ssl/test/SSLCertDataTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright (c) 2023 SUSE LLC
*
* This software is licensed to you under the GNU General Public License,
* version 2 (GPLv2). There is NO WARRANTY for this software, express or
* implied, including the implied warranties of MERCHANTABILITY or FITNESS
* FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
* along with this software; if not, see
* http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
*
* Red Hat trademarks are not licensed under GPLv2. No permission is
* granted to use or replicate Red Hat trademarks that are incorporated
* in this software or its documentation.
*/

package com.suse.manager.ssl.test;

import static org.junit.jupiter.api.Assertions.assertEquals;

import com.suse.manager.ssl.SSLCertData;
import com.suse.manager.webui.utils.gson.ProxyContainerConfigJson;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.stream.Stream;

public class SSLCertDataTest {
private SSLCertData sslCertData;

@BeforeEach
void setUp() {
sslCertData = new SSLCertData(null, null, null, null, null, null, null, null);
}



@ParameterizedTest
@MethodSource("machineNameTestData")
void testGetMachineNameWithCustomObject(SSLCertDataTestData sslCertDataTestDataIn) {
sslCertData = new SSLCertData(sslCertDataTestDataIn.getCn(), null, null, null, null, null, null, null);

String result = sslCertData.getMachineName();
assertEquals(sslCertDataTestDataIn.getExpectedMachineName(), result);
}


/**
* Test the getMachineName method.
* Note that cn pattern is matched at {@link ProxyContainerConfigJson}
*/
private static Stream<SSLCertDataTestData> machineNameTestData() {
return Stream.of(
new SSLCertDataTestData("xxx.yyy.zzz.com", "xxx.yyy"),
new SSLCertDataTestData("yyy.zzz.com", "yyy"),
new SSLCertDataTestData("zzz.com", "zzz.com"),
new SSLCertDataTestData("xxx", "xxx")
);
}

static class SSLCertDataTestData {
private final String cn;
private final String expectedMachineName;

SSLCertDataTestData(String cnIn, String expectedMachineNameIn) {
this.cn = cnIn;
this.expectedMachineName = expectedMachineNameIn;
}

String getCn() {
return cn;
}

String getExpectedMachineName() {
return expectedMachineName;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ public String generateContainerConfig(Request request, Response response, User u
return json(response, HttpStatus.SC_INTERNAL_SERVER_ERROR, msg);
}
catch (RhnRuntimeException e) {
LOG.error("Failed to generate proxy configuration", e);
return json(response, HttpStatus.SC_BAD_REQUEST, e.getMessage());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.util.List;
import java.util.Optional;
import java.util.regex.Pattern;

/**
* Represents the data sent from the UI to request a new proxy container configuration
Expand All @@ -30,6 +31,8 @@ public class ProxyContainerConfigJson {
private static final String CREATE_SSL = "create-ssl";
private static final String USE_SSL = "use-ssl";

private static final Pattern FQDN_PATTERN = Pattern.compile("^[A-Za-z0-9-]+(\\.[A-Za-z0-9-]+)*$");

@SerializedName("proxyFQDN")
private String proxyFqdn;
private Integer proxyPort;
Expand Down Expand Up @@ -69,7 +72,15 @@ public boolean isValid() {
else if (CREATE_SSL.equals(sslMode)) {
valid = caCert != null && caKey != null && caPassword != null;
}
return valid && proxyFqdn != null && serverFqdn != null && maxCache != null && email != null;
return valid && isValidFqdn(proxyFqdn) && isValidFqdn(serverFqdn) && maxCache != null && email != null;
}

/**
* @param fqdn the fqdn to check
* @return whether the fqdn is valid or not
*/
private boolean isValidFqdn(String fqdn) {
return fqdn != null && FQDN_PATTERN.matcher(fqdn).matches();
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- fix FQDN machine name mapping on proxy configuration
4 changes: 4 additions & 0 deletions web/html/src/manager/proxy/container-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ export function ProxyConfig() {
placeholder={t("e.g., proxy.domain.com")}
labelClass="col-md-3"
divClass="col-md-6"
validators={[Validation.matches(/^[A-Za-z0-9-]+(\.[A-Za-z0-9-]+)*$/)]}
invalidHint={t("Has to be a valid FQDN address")}
/>
<Text
name="serverFQDN"
Expand All @@ -211,6 +213,8 @@ export function ProxyConfig() {
hint={t("The FQDN of the parent (server or proxy) to connect to.")}
labelClass="col-md-3"
divClass="col-md-6"
validators={[Validation.matches(/^[A-Za-z0-9-]+(\.[A-Za-z0-9-]+)*$/)]}
invalidHint={t("Has to be a valid FQDN address")}
/>
<Text
name="proxyPort"
Expand Down
11 changes: 11 additions & 0 deletions web/spacewalk-web.changes.rjpmestre.fix_proxy_config_fqdn_mapping
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
- fix FQDN machine name mapping on proxy configuration
It was noticed that configuring a proxy sometimes would fail and just display a generic error.
Analysis can be split in two:
a) what was causing the problem
Files uploaded during configuration are stored and moved based on a 'machine name' mapping based on the provided 'Proxy FQDN'.
https://github.com/uyuni-project/uyuni/blob/32465f6efccd319926ae1897e14d9d20d4d89c39/spacewalk/certs-tools/sslToolLib.py#L73
Current solution based on a slightly more generic occurrence:
https://github.com/uyuni-project/uyuni/blob/f1f62028c363f8ea95b0cc080b4fa9eddf9fbaa0/spacewalk/certs-tools/rhn_ssl_tool.py#L205
b) why was just a generic error
This had to do with our (rhn) custom FileUtils throwing generic RTEs while RhnRuntimeException is expected
https://github.com/uyuni-project/uyuni/blob/32465f6efccd319926ae1897e14d9d20d4d89c39/java/code/src/com/redhat/rhn/common/util/FileUtils.java#L130

0 comments on commit a4071bf

Please sign in to comment.