Skip to content

Commit

Permalink
[PLAT-13392]Change the jsonPath mapping for sshPort for all the provi…
Browse files Browse the repository at this point in the history 
…der validations

Summary: As the `sshUser` and `sshPort` are moved to image bundle details level, the corresponding mappings should be changed for all the provider validations. Right now, they are mapping to `$.details.sshPort` instead it should map to `$.imageBundles[i].details.sshPort`. Changes are made for AWS and GCP. [Ssh port validation is not yet done for Azure]

Test Plan:
  - Tested the validation with `vm_os_patching`=`true` and `false` for both the `GCP` and `AWS` provider creation & edit

Reviewers: #yba-api-review!, svarshney

Reviewed By: svarshney

Subscribers: yugaware

Differential Revision: https://phorge.dev.yugabyte.com/D33796
  • Loading branch information
rohita committed Apr 4, 2024
1 parent 31be488 commit 39dc5c1
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 70 deletions.
102 changes: 71 additions & 31 deletions managed/src/main/java/com/yugabyte/yw/models/configs/validators/AWSProviderValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.yugabyte.yw.cloud.aws.AWSCloudImpl;
import com.yugabyte.yw.common.BeanValidator;
import com.yugabyte.yw.common.PlatformServiceException;
import com.yugabyte.yw.common.config.GlobalConfKeys;
import com.yugabyte.yw.common.config.RuntimeConfGetter;
import com.yugabyte.yw.models.AvailabilityZone;
import com.yugabyte.yw.models.ImageBundle;
Expand Down Expand Up @@ -73,6 +74,8 @@ public void validate(Provider provider) {
// Collect validation errors only when client is successfully created, otherwise,
// all other validations will unquestionably fail.
SetMultimap<String, String> validationErrorsMap = HashMultimap.create();
boolean enableVMOSPatching =
runtimeConfigGetter.getGlobalConf(GlobalConfKeys.enableVMOSPatching);

// validate SSH private key content
try {
Expand All @@ -96,10 +99,6 @@ public void validate(Provider provider) {
}
}

if (provider.getDetails().sshPort == null) {
validationErrorsMap.put("SSH_PORT", "Please provide a valid ssh port value");
}

// validate hosted zone id
if (provider.getRegions() != null && !provider.getRegions().isEmpty()) {
for (Region region : provider.getRegions()) {
Expand All @@ -121,9 +120,17 @@ public void validate(Provider provider) {
// validate Region and its details
if (provider.getRegions() != null && !provider.getRegions().isEmpty()) {
for (Region region : provider.getRegions()) {
validateAMI(provider, region, validationErrorsMap);
validateAMI(provider, region, validationErrorsMap, enableVMOSPatching);
validateVpc(provider, region, validationErrorsMap);
validateSgAndPort(provider, region, validationErrorsMap);
validateSg(provider, region, validationErrorsMap);
if (!enableVMOSPatching) {
String fieldDetails = "SSH_PORT";
Integer sshPort = provider.getDetails().getSshPort();
if (sshPort == null) {
validationErrorsMap.put("SSH_PORT", "Please provide a valid ssh port value");
}
validateSshPort(provider, region, fieldDetails, sshPort, validationErrorsMap);
}
validateSubnets(provider, region, validationErrorsMap);
dryRun(provider, region, validationErrorsMap);
}
Expand Down Expand Up @@ -183,15 +190,18 @@ private void dryRun(
}

private void validateAMI(
Provider provider, Region region, SetMultimap<String, String> validationErrorsMap) {
Provider provider,
Region region,
SetMultimap<String, String> validationErrorsMap,
boolean enableVMOSPatching) {
List<ImageBundle> imageBundles = provider.getImageBundles();
for (ImageBundle imageBundle : imageBundles) {
BundleInfo bundleInfo = imageBundle.getDetails().getRegions().get(region.getCode());
if (bundleInfo == null || StringUtils.isEmpty(bundleInfo.getYbImage())) {
continue;
}
String imageId = bundleInfo.getYbImage();
String fieldDetails = "REGION." + region.getCode() + ".IMAGE";
String fieldDetails = "REGION." + region.getCode() + ".IMAGE." + imageBundle.getName();
try {
Image image = awsCloudImpl.describeImageOrBadRequest(provider, region, imageId);
List<String> errorList = new ArrayList<>();
Expand All @@ -217,6 +227,15 @@ private void validateAMI(
if (errorList.size() != 0) {
validationErrorsMap.putAll(fieldDetails, errorList);
}
if (enableVMOSPatching) {
fieldDetails = fieldDetails + ".SSH_PORT";
Integer sshPort = imageBundle.getDetails().getSshPort();
if (sshPort == null) {
validationErrorsMap.put(fieldDetails, "Please provide a valid ssh port value");
continue;
}
validateSshPort(provider, region, fieldDetails, sshPort, validationErrorsMap);
}
} catch (PlatformServiceException e) {
if (e.getHttpStatus() == BAD_REQUEST) {
validationErrorsMap.put(fieldDetails, e.getMessage());
Expand All @@ -243,7 +262,7 @@ private void validateVpc(
}
}

private void validateSgAndPort(
private void validateSg(
Provider provider, Region region, SetMultimap<String, String> validationErrorsMap) {
String fieldDetails = "REGION." + region.getCode() + ".SECURITY_GROUP";

Expand All @@ -259,28 +278,6 @@ private void validateSgAndPort(
errorList.add(
securityGroup.getGroupId() + " is not attached to vpc: " + region.getVnetName());
}
Integer sshPort = provider.getDetails().getSshPort();
boolean portOpen = false;
if (!CollectionUtils.isNullOrEmpty(securityGroup.getIpPermissions())) {
for (IpPermission ipPermission : securityGroup.getIpPermissions()) {
Integer fromPort = ipPermission.getFromPort();
Integer toPort = ipPermission.getToPort();
if (fromPort == null && toPort == null) {
portOpen = true;
break;
}
if (fromPort == null || toPort == null) {
continue;
}
if (fromPort <= sshPort && toPort >= sshPort) {
portOpen = true;
break;
}
}
}
if (!portOpen) {
errorList.add(sshPort + " is not open on security group " + securityGroup.getGroupId());
}
}
if (errorList.size() != 0) {
validationErrorsMap.putAll(fieldDetails, errorList);
Expand All @@ -295,6 +292,49 @@ private void validateSgAndPort(
}
}

private void validateSshPort(
Provider provider,
Region region,
String fieldDetails,
Integer sshPort,
SetMultimap<String, String> validationErrorsMap) {
try {
List<SecurityGroup> securityGroupList =
awsCloudImpl.describeSecurityGroupsOrBadRequest(provider, region);
for (SecurityGroup securityGroup : securityGroupList) {
boolean portOpen = false;
if (!CollectionUtils.isNullOrEmpty(securityGroup.getIpPermissions())) {
for (IpPermission ipPermission : securityGroup.getIpPermissions()) {
Integer fromPort = ipPermission.getFromPort();
Integer toPort = ipPermission.getToPort();
if (fromPort == null && toPort == null) {
portOpen = true;
break;
}
if (fromPort == null || toPort == null) {
continue;
}
if (fromPort <= sshPort && toPort >= sshPort) {
portOpen = true;
break;
}
}
}
if (!portOpen) {
String errorMsg =
sshPort + " is not open on security group " + securityGroup.getGroupId();
validationErrorsMap.put(fieldDetails, errorMsg);
}
}
} catch (PlatformServiceException e) {
if (e.getHttpStatus() == BAD_REQUEST) {
validationErrorsMap.put(fieldDetails, e.getMessage());
} else {
throw e;
}
}
}

private void validateSubnets(
Provider provider, Region region, SetMultimap<String, String> validationErrorsMap) {
String fieldDetails = "REGION." + region.getCode() + ".SUBNETS";
Expand Down
69 changes: 50 additions & 19 deletions managed/src/main/java/com/yugabyte/yw/models/configs/validators/GCPProviderValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public void validate(Provider provider) {
GCPProjectApiClient apiClient = new GCPProjectApiClient(runtimeConfGetter, provider);
ArrayNode regionArrayJson = (ArrayNode) processedProvider.get("regions");
ArrayNode imageBundleArrayJson = (ArrayNode) processedProvider.get("imageBundles");
boolean enableVMOSPatching = runtimeConfGetter.getGlobalConf(GlobalConfKeys.enableVMOSPatching);

// Verify role bindings [if the SA has req permissions to manage instances]
boolean validateNewVpcPerms = getVpcType(provider) == VPCType.NEW;
Expand All @@ -84,15 +85,19 @@ public void validate(Provider provider) {
validateRegions(provider, apiClient, validationErrorsMap, regionArrayJson);

// Validate imageBundles
validateImageBundles(provider, apiClient, validationErrorsMap, imageBundleArrayJson);
validateImageBundles(
provider, apiClient, validationErrorsMap, imageBundleArrayJson, enableVMOSPatching);

// Validate if the keypair is an RSA keypair
validatePrivateKeys(provider, processedProvider, validationErrorsMap);

if (provider.getDetails() != null) {
// Verify if the ssh port is open
validateSshPort(provider, apiClient, validationErrorsMap, detailsJson);

if (!enableVMOSPatching && provider.getDetails().getSshPort() != null) {
// Verify if the ssh port is open
String jsonPath = detailsJson.get("sshPort").get("jsonPath").asText();
int sshPort = provider.getDetails().getSshPort();
validateSshPort(provider, apiClient, validationErrorsMap, sshPort, jsonPath);
}
// Verify the existence of the tag to any of the firewall rules.
validateFirewallTags(provider, apiClient, validationErrorsMap, cloudInfoJson);

Expand Down Expand Up @@ -234,7 +239,8 @@ private void validateImageBundles(
Provider provider,
GCPProjectApiClient apiClient,
SetMultimap<String, String> validationErrorsMap,
JsonNode imageBundleArrayJson) {
JsonNode imageBundleArrayJson,
boolean enableVMOSPatching) {
List<ImageBundle> imageBundles = provider.getImageBundles();
int imageInd = 0;
for (ImageBundle imageBundle : imageBundles) {
Expand All @@ -250,6 +256,19 @@ private void validateImageBundles(
} catch (PlatformServiceException e) {
validationErrorsMap.put(jsonPath, e.getMessage());
}
try {
if (enableVMOSPatching) {
// Verify if the ssh port is open
if (imageBundle.getDetails().getSshPort() == null) {
continue;
}
int sshPort = imageBundle.getDetails().getSshPort();
jsonPath = imageBundleJson.get("details").get("sshPort").get("jsonPath").asText();
validateSshPort(provider, apiClient, validationErrorsMap, sshPort, jsonPath);
}
} catch (PlatformServiceException e) {
validationErrorsMap.put(jsonPath, e.getMessage());
}
}
}

Expand All @@ -273,9 +292,9 @@ private void validateSshPort(
Provider provider,
GCPProjectApiClient apiClient,
SetMultimap<String, String> validationErrorsMap,
JsonNode detailsJson) {
if (provider.getDetails().getSshPort() != null && getVpcType(provider) != VPCType.NEW) {
String jsonPath = detailsJson.get("sshPort").get("jsonPath").asText();
int sshPort,
String jsonPath) {
if (getVpcType(provider) != VPCType.NEW) {
try {
String vpcNetwork = getVpcNetwork(provider);
String vpcProject = getVpcProject(provider);
Expand All @@ -302,12 +321,13 @@ private void validateSshPort(
policy,
provider,
networkSelfLink,
networkFirewallPolicyEnforcementOrder)) {
networkFirewallPolicyEnforcementOrder,
sshPort)) {
errorMsg =
String.format(
"Connection(ssh) to host will fail at port %d as the port is not opened on any"
+ " firewall",
provider.getDetails().getSshPort());
sshPort);
validationErrorsMap.put(jsonPath, errorMsg);
}
} else {
Expand All @@ -317,12 +337,13 @@ private void validateSshPort(
policy,
provider,
networkSelfLink,
networkFirewallPolicyEnforcementOrder)) {
networkFirewallPolicyEnforcementOrder,
sshPort)) {
errorMsg =
String.format(
"Connection(ssh) to host will fail at port %d as the port is not opened on any"
+ " firewall",
provider.getDetails().getSshPort());
sshPort);
validationErrorsMap.put(jsonPath, errorMsg);
}
}
Expand Down Expand Up @@ -416,7 +437,8 @@ public boolean checkSshPortFirewallRules(
FirewallPolicy policy,
Provider provider,
String networkSelfLink,
String networkFirewallPolicyEnforcementOrder) {
String networkFirewallPolicyEnforcementOrder,
int sshPort) {
if (!firewallRules.isEmpty()) {
// If ybFirewallTags are not provided, "cluster-server" is the default tag that is added for
// GCP instances.
Expand All @@ -432,7 +454,6 @@ public boolean checkSshPortFirewallRules(
.collect(Collectors.toList());

if (!filteredRules.isEmpty()) {
int sshPort = provider.getDetails().getSshPort();
for (Firewall firewallRule : filteredRules) {
// Check if any rule explicitly denies sshPort/TCP
if (firewallDeniedSshPort(firewallRule, sshPort)) {
Expand All @@ -449,7 +470,12 @@ public boolean checkSshPortFirewallRules(
}
if (networkFirewallPolicyEnforcementOrder.equals(GCPUtil.ENFORCEMENT_AFTER_CLASSIC_FIREWALL)) {
return checkSshPortFirewallPolicy(
firewallRules, policy, provider, networkSelfLink, networkFirewallPolicyEnforcementOrder);
firewallRules,
policy,
provider,
networkSelfLink,
networkFirewallPolicyEnforcementOrder,
sshPort);
}
return false;
}
Expand Down Expand Up @@ -504,7 +530,8 @@ public boolean checkSshPortFirewallPolicy(
FirewallPolicy policy,
Provider provider,
String networkSelfLink,
String networkFirewallPolicyEnforcementOrder) {
String networkFirewallPolicyEnforcementOrder,
int sshPort) {
if (policy != null) {
// Filter the rules of the firewall policy to select only the ingress rules that are enabled;
// targetResources == null || contains networkurl
Expand All @@ -530,8 +557,7 @@ public boolean checkSshPortFirewallPolicy(
|| config.getIpProtocol().equalsIgnoreCase("tcp"))
&& (config.getPorts() == null
|| config.getPorts().isEmpty()
|| isValidSshPort(
config.getPorts(), provider.getDetails().getSshPort())));
|| isValidSshPort(config.getPorts(), sshPort)));
if (ipPortMatched) {
// If tcp/sshPort matched, return false if action is denied
return !rule.getAction().equalsIgnoreCase("deny");
Expand All @@ -541,7 +567,12 @@ public boolean checkSshPortFirewallPolicy(
}
if (networkFirewallPolicyEnforcementOrder.equals(GCPUtil.ENFORCEMENT_BEFORE_CLASSIC_FIREWALL)) {
return checkSshPortFirewallRules(
firewallRules, policy, provider, networkSelfLink, networkFirewallPolicyEnforcementOrder);
firewallRules,
policy,
provider,
networkSelfLink,
networkFirewallPolicyEnforcementOrder,
sshPort);
}
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion managed/src/test/java/com/yugabyte/yw/commissioner/tasks/CloudProviderEditTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ public void testValidationOKToError() throws InterruptedException {
assertNotNull(provider.getLastValidationErrors());
assertEquals(
Json.parse("[\"AMI details extraction failed: Not found\"]"),
provider.getLastValidationErrors().get("error").get("REGION.us-west-1.IMAGE"));
provider.getLastValidationErrors().get("error").get("REGION.us-west-1.IMAGE.test-image"));
assertEquals(Provider.UsabilityState.READY, provider.getUsabilityState());
assertEquals("new name", provider.getName());
}
Expand Down
Loading

0 comments on commit 39dc5c1

Please sign in to comment.