Skip to content

Commit

Permalink
OADP-378 cherry-pick 1.0
Browse files Browse the repository at this point in the history 
OADP-378 cherry-pick 1.0

Fix access key value containing `=` edge case (openshift#603)

* Fix secret `=` edge case

* revert `-d` for go-get-tool

* remove `=` from azure match key

* add unit test case with `=` in secretkey value

* allow quotes and spaces + tests

* Remove space via replaceAll instead of trim
  • Loading branch information
@kaovilai
kaovilai committed May 6, 2022
1 parent b421de7 commit 6e41423
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 31 deletions.
5 changes: 3 additions & 2 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ ENVTEST_K8S_VERSION = 1.21

.PHONY:ginkgo
ginkgo: # Make sure ginkgo is in $GOPATH/bin
go get github.com/onsi/ginkgo/ginkgo
go get github.com/onsi/gomega/...
go get -d github.com/onsi/ginkgo/ginkgo
go get -d github.com/onsi/ginkgo/v2/ginkgo
go get -d github.com/onsi/gomega/...

# VERSION defines the project version for the bundle.
# Update this value when you upgrade the version of your project.
Expand Down
1 change: 0 additions & 1 deletion controllers/bucket_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ func (b BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
if err != nil {
return result, err
}

var ok bool
if ok, err = clnt.Exists(); !ok {
// Handle Deletion.
Expand Down
60 changes: 32 additions & 28 deletions controllers/registry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,14 +575,18 @@ func (r *DPAReconciler) parseAWSSecret(secret corev1.Secret, secretKey string, m
AWSAccessKey, AWSSecretKey, profile := "", "", ""
splitString := strings.Split(string(secret.Data[secretKey]), "\n")
keyNameRegex, err := regexp.Compile(`\[.*\]`)
const (
accessKeyKey = "aws_access_key_id"
secretKeyKey = "aws_secret_access_key"
)
if err != nil {
return AWSAccessKey, AWSSecretKey, errors.New("parseAWSSecret faulty regex: keyNameRegex")
}
awsAccessKeyRegex, err := regexp.Compile(`\baws_access_key_id\b`)
awsAccessKeyRegex, err := regexp.Compile(`\b` + accessKeyKey + `\b`)
if err != nil {
return AWSAccessKey, AWSSecretKey, errors.New("parseAWSSecret faulty regex: awsAccessKeyRegex")
}
awsSecretKeyRegex, err := regexp.Compile(`\baws_secret_access_key\b`)
awsSecretKeyRegex, err := regexp.Compile(`\b` + secretKeyKey + `\b`)
if err != nil {
return AWSAccessKey, AWSSecretKey, errors.New("parseAWSSecret faulty regex: awsSecretKeyRegex")
}
Expand Down Expand Up @@ -615,22 +619,18 @@ func (r *DPAReconciler) parseAWSSecret(secret corev1.Secret, secretKey string, m
return AWSAccessKey, AWSSecretKey, err
}
if matchedAccessKey { // check for access key
cleanedLine := strings.ReplaceAll(profLine, " ", "")
splitLine := strings.Split(cleanedLine, "=")
if len(splitLine) != 2 {
r.Log.Info("Could not parse secret for AWS Access key")
return AWSAccessKey, AWSSecretKey, errors.New("secret parsing error")
AWSAccessKey, err = r.getMatchedKeyValue(accessKeyKey, profLine)
if err != nil {
r.Log.Info("Error processing access key id for the supplied AWS credential")
return AWSAccessKey, AWSSecretKey, err
}
AWSAccessKey = splitLine[1]
continue
} else if matchedSecretKey { // check for secret key
cleanedLine := strings.ReplaceAll(profLine, " ", "")
splitLine := strings.Split(cleanedLine, "=")
if len(splitLine) != 2 {
r.Log.Info("Could not parse secret for AWS Secret key")
return AWSAccessKey, AWSSecretKey, errors.New("secret parsing error")
AWSSecretKey, err = r.getMatchedKeyValue(secretKeyKey, profLine)
if err != nil {
r.Log.Info("Error processing secret key id for the supplied AWS credential")
return AWSAccessKey, AWSSecretKey, err
}
AWSSecretKey = splitLine[1]
continue
} else {
break // aws credentials file is only allowed to have profile followed by aws_access_key_id, aws_secret_access_key
Expand Down Expand Up @@ -706,37 +706,37 @@ func (r *DPAReconciler) parseAzureSecret(secret corev1.Secret, secretKey string)

switch {
case matchedStorageKey:
storageKeyValue, err := r.getMatchedKeyValue("AZURE_STORAGE_ACCOUNT_ACCESS_KEY=", line)
storageKeyValue, err := r.getMatchedKeyValue("AZURE_STORAGE_ACCOUNT_ACCESS_KEY", line)
if err != nil {
return azcreds, err
}
azcreds.strorageAccountKey = storageKeyValue
case matchedSubscriptionId:
subscriptionIdValue, err := r.getMatchedKeyValue("AZURE_SUBSCRIPTION_ID=", line)
subscriptionIdValue, err := r.getMatchedKeyValue("AZURE_SUBSCRIPTION_ID", line)
if err != nil {
return azcreds, err
}
azcreds.subscriptionID = subscriptionIdValue
case matchedCliendId:
clientIdValue, err := r.getMatchedKeyValue("AZURE_CLIENT_ID=", line)
clientIdValue, err := r.getMatchedKeyValue("AZURE_CLIENT_ID", line)
if err != nil {
return azcreds, err
}
azcreds.clientID = clientIdValue
case matchedClientsecret:
clientSecretValue, err := r.getMatchedKeyValue("AZURE_CLIENT_SECRET=", line)
clientSecretValue, err := r.getMatchedKeyValue("AZURE_CLIENT_SECRET", line)
if err != nil {
return azcreds, err
}
azcreds.clientSecret = clientSecretValue
case matchedResourceGroup:
resourceGroupValue, err := r.getMatchedKeyValue("AZURE_RESOURCE_GROUP=", line)
resourceGroupValue, err := r.getMatchedKeyValue("AZURE_RESOURCE_GROUP", line)
if err != nil {
return azcreds, err
}
azcreds.resourceGroup = resourceGroupValue
case matchedTenantId:
tenantIdValue, err := r.getMatchedKeyValue("AZURE_TENANT_ID=", line)
tenantIdValue, err := r.getMatchedKeyValue("AZURE_TENANT_ID", line)
if err != nil {
return azcreds, err
}
Expand All @@ -746,15 +746,19 @@ func (r *DPAReconciler) parseAzureSecret(secret corev1.Secret, secretKey string)
return azcreds, nil
}

func (r *DPAReconciler) getMatchedKeyValue(matchedKey string, line string) (string, error) {
cleanedLine := strings.ReplaceAll(line, " ", "")
cleanedLine = strings.ReplaceAll(cleanedLine, "\"", "")
matchedKeyValue := strings.Replace(cleanedLine, matchedKey, "", -1)
if len(matchedKeyValue) == 0 {
r.Log.Info("Could not parse secret for %s", matchedKey)
return matchedKeyValue, errors.New("azure secret parsing error")
// Return value to the right of = sign with quotations and spaces removed.
func (r *DPAReconciler) getMatchedKeyValue(key string, s string) (string, error) {
for _, removeChar := range []string{"\"", "'", " "} {
s = strings.ReplaceAll(s, removeChar, "")
}
return matchedKeyValue, nil
for _, prefix := range []string{key, "="} {
s = strings.TrimPrefix(s, prefix)
}
if len(s) == 0 {
r.Log.Info("Could not parse secret for %s", key)
return s, errors.New(key + " secret parsing error")
}
return s, nil
}

func (r *DPAReconciler) ReconcileRegistrySVCs(log logr.Logger) (bool, error) {
Expand Down
126 changes: 126 additions & 0 deletions controllers/registry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,32 @@ var (
"aws_secret_access_key=" + testSecretAccessKey,
),
}
secretDataWithEqualInSecret = map[string][]byte{
"cloud": []byte(
"\n[" + testBslProfile + "]\n" +
"aws_access_key_id=" + testBslAccessKey + "\n" +
"aws_secret_access_key=" + testBslSecretAccessKey + "=" + testBslSecretAccessKey +
"\n[default]" + "\n" +
"aws_access_key_id=" + testAccessKey + "\n" +
"aws_secret_access_key=" + testSecretAccessKey + "=" + testSecretAccessKey +
"\n[test-profile]\n" +
"aws_access_key_id=" + testAccessKey + "\n" +
"aws_secret_access_key=" + testSecretAccessKey + "=" + testSecretAccessKey,
),
}
secretDataWithMixedQuotesAndSpacesInSecret = map[string][]byte{
"cloud": []byte(
"\n[" + testBslProfile + "]\n" +
"aws_access_key_id =" + testBslAccessKey + "\n" +
" aws_secret_access_key=" + "\" " + testBslSecretAccessKey + "\"" +
"\n[default]" + "\n" +
" aws_access_key_id= " + testAccessKey + "\n" +
"aws_secret_access_key =" + "'" + testSecretAccessKey + " '" +
"\n[test-profile]\n" +
"aws_access_key_id =" + testAccessKey + "\n" +
"aws_secret_access_key=" + "\" " + testSecretAccessKey + "\"",
),
}
awsSecretDataWithMissingProfile = map[string][]byte{
"cloud": []byte(
"[default]" + "\n" +
Expand Down Expand Up @@ -185,6 +211,106 @@ func TestDPAReconciler_buildRegistryDeployment(t *testing.T) {
},
dpa: &oadpv1alpha1.DataProtectionApplication{},
},
{
name: "given a valid bsl with equal in secret val get appropriate registry deployment",
registryDeployment: &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "test-registry",
Namespace: "test-ns",
},
Spec: appsv1.DeploymentSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"component": "oadp-" + "test-bsl" + "-" + "aws" + "-registry",
},
},
},
},
bsl: &velerov1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bsl",
Namespace: "test-ns",
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: AWSProvider,
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "aws-bucket",
},
},
Config: map[string]string{
Region: "aws-region",
S3URL: "https://sr-url-aws-domain.com",
InsecureSkipTLSVerify: "false",
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials",
Namespace: "test-ns",
},
Data: secretDataWithEqualInSecret,
},
registrySecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "oadp-test-bsl-aws-registry-secret",
Namespace: "test-ns",
},
Data: awsRegistrySecretData,
},
dpa: &oadpv1alpha1.DataProtectionApplication{},
},
{
name: "given a valid bsl with use of quotes in secret val get appropriate registry deployment",
registryDeployment: &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "test-registry",
Namespace: "test-ns",
},
Spec: appsv1.DeploymentSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"component": "oadp-" + "test-bsl" + "-" + "aws" + "-registry",
},
},
},
},
bsl: &velerov1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bsl",
Namespace: "test-ns",
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: AWSProvider,
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "aws-bucket",
},
},
Config: map[string]string{
Region: "aws-region",
S3URL: "https://sr-url-aws-domain.com",
InsecureSkipTLSVerify: "false",
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials",
Namespace: "test-ns",
},
Data: secretDataWithMixedQuotesAndSpacesInSecret,
},
registrySecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "oadp-test-bsl-aws-registry-secret",
Namespace: "test-ns",
},
Data: awsRegistrySecretData,
},
dpa: &oadpv1alpha1.DataProtectionApplication{},
},
{
name: "given a valid bsl with velero annotation get appropriate registry deployment",
registryDeployment: &appsv1.Deployment{
Expand Down

0 comments on commit 6e41423

Please sign in to comment.