-
Notifications
You must be signed in to change notification settings - Fork 69
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
Replace carriage return when getting provider secrets for registry + E2E changes #652
Replace carriage return when getting provider secrets for registry + E2E changes #652
Conversation
a7788f6
to
c3453f1
Compare
@@ -499,13 +533,23 @@ var _ = Describe("Configuration testing for DPA Custom Resource", func() { | |||
if installCase.DpaSpec.BackupLocations[0].Velero.Config != nil { | |||
installCase.DpaSpec.BackupLocations[0].Velero.Config["credentialsFile"] = "bsl-cloud-credentials-aws/cloud" | |||
} | |||
if installCase.TestCarriageReturn { | |||
installCase.DpaSpec.BackupLocations[0].Velero.Config["credentialsFile"] = "bsl-cloud-credentials-aws-with-carriage-return/cloud" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using carriage return secret
Entry("Default velero CR, test carriage return", InstallCase{ | ||
Name: "default-cr", | ||
BRestoreType: RESTIC, | ||
TestCarriageReturn: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a test for carriage return case
@@ -37,3 +38,9 @@ func WriteFile(credFile string, data []byte) error { | |||
err := ioutil.WriteFile(credFile, data, 0644) | |||
return err | |||
} | |||
|
|||
func ReplaceSecretDataNewLineWithCarriageReturn(data []byte) []byte { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New function to replace newline with carriage return in secret data.
tests/e2e/e2e_suite_test.go
Outdated
@@ -103,6 +107,8 @@ var _ = BeforeSuite(func() { | |||
cloudCreds := GetAzureCreds(cloudCredData) | |||
err = CreateCredentialsSecret(cloudCreds, namespace, "bsl-cloud-credentials-azure") | |||
Expect(err).NotTo(HaveOccurred()) | |||
err = CreateCredentialsSecret(utils.ReplaceSecretDataNewLineWithCarriageReturn(cloudCreds), namespace, "bsl-cloud-credentials-azure-with-carriage-return") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create another secret to use to test carriage return case.
Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com>
…into registryCanUseOtherSecretNameFor_credentialsFile
c3453f1
to
7ea4fa4
Compare
/retest |
1 similar comment
/retest |
@kaovilai: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the tests
@@ -33,7 +33,6 @@ var ( | |||
IsCloudProvider: true, | |||
SecretName: "cloud-credentials", | |||
MountPath: "/credentials", | |||
BslSecretName: "bsl-cloud-credentials-aws", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kinda followed the logic we had for the previous ones as I included this for the case of CI, now that we are pushing this for new feature, we expect the user to have created the secret name with key given in the credentialsFile
right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think user should be able to pick which secret is used. My change didn't break functionality so I think it's okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prior to this PR, user was expected to have created a secret anyway but the name that they can use when specifying credentialsFile was hardcoded. This meant I couldn't use a different secret for the carriage return test case unless I overwrite the current secret with the fixed name.
This only affected the credentialFile config case which most customers won't be using. With this PR the secret name for registry is now flexible if they specified bsl secret with credentialsFile config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we expect the user to have created the secret name with key given in the credentialsFile right?
Yes.
…E2E changes (openshift#652) * Replace carriage return when getting provider secrets for registry Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * add tests * replaceCarriageReturn unit test * Registry credentials can use other secret names Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com>
…E2E changes (openshift#652) * Replace carriage return when getting provider secrets for registry Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * add tests * replaceCarriageReturn unit test * Registry credentials can use other secret names Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com>
…E2E changes (openshift#652) * Replace carriage return when getting provider secrets for registry Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * add tests * replaceCarriageReturn unit test * Registry credentials can use other secret names Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com>
…E2E changes (openshift#652) * Replace carriage return when getting provider secrets for registry Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * add tests * replaceCarriageReturn unit test * Registry credentials can use other secret names Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com>
…n updateRegistrySecret (#679) * Replace carriage return when getting provider secrets for registry + E2E changes (#652) * Replace carriage return when getting provider secrets for registry Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * add tests * replaceCarriageReturn unit test * Registry credentials can use other secret names Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * Multi cloud E2E Updates (#568) * Adding changes from ci-multi-cloud branch * Updating flags in makefile * Updating flags in makefile * Updating flags in makefile * Update defaults * Fix helpers * Cleaning up makefile * Update makefile * Update kustomization * Cleaning up Makefile * Adding changes for AWS CredentialFile * Handling no default backuplocations * Removing comments * Adding config check in helpers * Fix AWS Test case * Fix basic review comments * genericTests... * Changing Credentials Mount Name for BSL when CredentialsFile key is used in Config - Fixes CI (#637) * Changing Credentials Mount Name for BSL for GCP * Fix DefaultPlugin for credentialsFile * E2E Tests: Fix azure templating name. (#573) * Fixing Azure to run locally * Aligning json * Handling Azure Parameters in CI Env using templates. (#582) * Adding azure parameters * Adding Openshift CI params * Update Test Suite and Helpers * Adding cred ref * Adding cred ref * Changing azure oadp cred dir to tmp * Fixing duplicate error * Removing OpenShift CI * Changing test instance name * Changing the AWS BSL Profile to default * Changing BslMountPath variable in registry controller * Replace carriage return when getting provider secrets for registry + E2E changes (#652) * Replace carriage return when getting provider secrets for registry Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * add tests * replaceCarriageReturn unit test * Registry credentials can use other secret names Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> * fix missing err return for updateRegistrySecret function * go fmt * upgraded ginkgo on tests to ginkgo 2.0 (#560) Co-authored-by: Nitish Srivastava <43022982+nitishSr@users.noreply.github.com> Co-authored-by: GitHub Co-pilot <idkwhatemailgithubcopilotuses@github.com> Co-authored-by: Deepak Raj D S <drajds@redhat.com> Co-authored-by: Shubham Pampattiwar <shubhampampattiwar7@gmail.com> Co-authored-by: Maya Peretz <mperetz@redhat.com>
OADP-463
Close #621