Skip to content

Commit

Permalink
[PLAT-13266] allow underscore in gcs bucket
Browse files Browse the repository at this point in the history
Summary:
GCS allows underscore in bucket name while S3 and Azure storage don't.
We validate buckets using [[ https://github.com/apache/commons-validator/blob/master/src/main/java/org/apache/commons/validator/UrlValidator.java | UrlValidator]].
The validation fails when there is a underscore in the gcs bucket since hostnames are not allowed to have underscores.

To fix this we use custom regex for GCS bucket validation. The regex follows the rules for bucket name mentioned [[ https://cloud.google.com/storage/docs/buckets#naming | here ]] .

Test Plan:
Manually verified that we can create GCS backup config with underscore in the bucket.
Added test cases to existing UT.

Reviewers: #yba-api-review!, vpatibandla

Reviewed By: vpatibandla

Subscribers: yugaware

Differential Revision: https://phorge.dev.yugabyte.com/D34901
  • Loading branch information
asharma-yb committed May 13, 2024
1 parent ea27a38 commit d0ce45c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public class CustomerConfigStorageGCSValidator extends CustomerConfigStorageVali
private static final Collection<String> GCS_URL_SCHEMES =
Arrays.asList(new String[] {"https", "gs"});

private static final String AUTHORITY_CHARS_REGEX = "^[a-z0-9][a-z0-9._-]{1,}[a-z0-9]$";

private final CloudClientsFactory factory;
private final GCPUtil gcpUtil;

Expand All @@ -38,7 +40,7 @@ public class CustomerConfigStorageGCSValidator extends CustomerConfigStorageVali
@Inject
public CustomerConfigStorageGCSValidator(
BeanValidator beanValidator, CloudClientsFactory factory, GCPUtil gcpUtil) {
super(beanValidator, GCS_URL_SCHEMES);
super(beanValidator, GCS_URL_SCHEMES, AUTHORITY_CHARS_REGEX);
this.factory = factory;
this.gcpUtil = gcpUtil;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import javax.inject.Inject;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.validator.routines.DomainValidator;
import org.apache.commons.validator.routines.RegexValidator;
import org.apache.commons.validator.routines.UrlValidator;

public class CustomerConfigStorageValidator extends ConfigDataValidator {
Expand All @@ -29,13 +30,25 @@ public class CustomerConfigStorageValidator extends ConfigDataValidator {
private final UrlValidator urlValidator;

@Inject
public CustomerConfigStorageValidator(BeanValidator beanValidator, Collection<String> schemes) {
public CustomerConfigStorageValidator(
BeanValidator beanValidator, Collection<String> schemes, String authRegex) {
super(beanValidator);

DomainValidator domainValidator = DomainValidator.getInstance(true);
RegexValidator authorityValidator = null;
if (authRegex != null) {
authorityValidator = new RegexValidator(authRegex);
}
urlValidator =
new UrlValidator(
schemes.toArray(new String[0]), null, UrlValidator.ALLOW_LOCAL_URLS, domainValidator);
schemes.toArray(new String[0]),
authorityValidator,
UrlValidator.ALLOW_LOCAL_URLS,
domainValidator);
}

public CustomerConfigStorageValidator(BeanValidator beanValidator, Collection<String> schemes) {
this(beanValidator, schemes, null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ public void setUp() {
"S3, BACKUP_LOCATION,, false",
"GCS, BACKUP_LOCATION, gs://itest-backup, true",
"GCS, BACKUP_LOCATION, gs://itest-backup/test, true",
"GCS, BACKUP_LOCATION, gs://itest_backup, true",
"GCS, BACKUP_LOCATION, gs://itest_backup_1, true",
"GCS, BACKUP_LOCATION, gs://itest_backup_, false",
"GCS, BACKUP_LOCATION, https://storage.googleapis.com/itest-backup/test, true",
"GCS, BACKUP_LOCATION, gcp.test.com, false",
"GCS, BACKUP_LOCATION, ftp://gcp.test.com, false",
Expand Down

0 comments on commit d0ce45c

Please sign in to comment.