Closed mateusoliveira43 closed 1 month ago
@mateusoliveira43: This pull request references OADP-4935 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.
re: issue around using secret for something other than object storage access key https://github.com/vmware-tanzu/velero/issues/7767
but it's not a current concern, something if impacted can be addressed later.
weird.. I see no change to the cred's permssions. Perhaps I'm doing something wrong?
-5.1$ cd /tmp/credentials/openshift-adp/
sh-5.1$ ls -la
total 4
drwxr-xr-x. 2 1000720000 root 37 Oct 3 20:05 .
drwxr-xr-x. 3 1000720000 root 27 Oct 3 20:05 ..
-rw-r--r--. 1 1000720000 root 116 Oct 3 20:08 cloud-credentials-cloud
sh-5.1$
weird.. I see no change to the cred's permssions. Perhaps I'm doing something wrong?
* make deploy-olm w/ * [46c1b46](https://github.com/openshift/oadp-operator/commit/46c1b467bad14d0266731bd4e474d7c944dde123) - (HEAD -> fix/default-permission) fixup! fix: Default permission for Secrets (9 hours ago) * create dpa w/ creds of course * rsh into velero pod
-5.1$ cd /tmp/credentials/openshift-adp/ sh-5.1$ ls -la total 4 drwxr-xr-x. 2 1000720000 root 37 Oct 3 20:05 . drwxr-xr-x. 3 1000720000 root 27 Oct 3 20:05 .. -rw-r--r--. 1 1000720000 root 116 Oct 3 20:08 cloud-credentials-cloud sh-5.1$
@weshayutin I believe you are checking the perms for the traditional/velero BSL creds via Bring your own bucket workflow. I think the JIRA is regarding the creds mounted for the BSL created via CloudStorage/Bucket workflow where we create the bucket and create a BSL using that bucket. Mount points would be different in both cases.
@shubham-pampattiwar look at the bug https://issues.redhat.com/browse/OADP-4935 The bug does not reference, what type of storage location ( bsl, vsl ), it's referring only to mounted secrets. AFAICT this patch has not fixed the jira issue yet.
@weshayutin @shubham-pampattiwar for this to work, you can not set dpa.spec.backupLocation.credentials
In my test (but it is wrong still, it is 0440
and it is set as 0400
in code)
sh-5.1$ ls -1la credentials/cloud
lrwxrwxrwx. 1 root 1000660000 12 Oct 4 13:53 credentials/cloud -> ..data/cloud
sh-5.1$ ls -1laL credentials/cloud
-r--r-----. 1 root 1000660000 132 Oct 4 13:53 credentials/cloud
sh-5.1$ ls -1a tmp/credentials/test-oadp-operator/
.
..
@mateusoliveira43 Hmm. For that last example, that's root-owned -- so the velero pod really needs the group read to be able to access it since it doesn't run as root.
@mateusoliveira43: This pull request references OADP-4935 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.
@sseago updated from 0400
to 0440
/lgtm
/hold lets test more just to be sure, before merging?
it won't merge until all required tests green
the lgtm will just enable auto retests (if there is no hold)
my worries here is that E2E does not test these changes, as I added in PR description
@kaovilai @sseago so when I pull this and get on the cluster even w/ my standard dpa that I used previously .. I will see the permissions changed?
@weshayutin if you specify credentials in BSL (even if the name is the default one), no
added info in PR description
@mateusoliveira43 In your opinion does that fix the jira issue?
ok.. lgtm to w/ my default dpa, same one I tested before
sh-5.1$
sh-5.1$ ls -la /credentials/cloud
lrwxrwxrwx. 1 root 1000720000 12 Oct 4 19:55 /credentials/cloud -> ..data/cloud
sh-5.1$ ls -la /credentials/..data/cloud
-r--r-----. 1 root 1000720000 116 Oct 4 19:55 /credentials/..data/cloud
sh-5.1$
spec:
backupLocations:
- velero:
config:
profile: default
region: us-west-2
credential:
key: cloud
name: cloud-credentials
default: true
objectStorage:
bucket: cvpbucketuswest2
prefix: velero
provider: aws
configuration:
nodeAgent:
enable: true
uploaderType: kopia
velero:
defaultPlugins:
- kubevirt
- csi
- openshift
- aws
snapshotLocations:
- velero:
config:
profile: default
region: us-west-2
provider: aws
@mateusoliveira43: This pull request references OADP-4935 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.
@mateusoliveira43: This pull request references OADP-4935 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.
@mateusoliveira43: This pull request references OADP-4935 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.
@mateusoliveira43: This pull request references OADP-4935 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.
Tested BSL (WITHOUT credentials) by running modified E2E tests locally, tests passed. Did not test VSL
spampatt@spampatt-mac oadp-operator [secret-read-only]$ oc exec -it velero-7758ddd869-fnqn7 -- /bin/sh
Defaulted container "velero" out of: velero, openshift-velero-plugin (init), velero-plugin-for-aws (init)
sh-5.1$ ls -la credentials/cloud
lrwxrwxrwx. 1 root root 12 Oct 8 00:17 credentials/cloud -> ..data/cloud
sh-5.1$ ls -lalL credentials/cloud
-r--r-----. 1 root root 116 Oct 8 00:17 credentials/cloud
sh-5.1$ ls -lalL tmp/credentials/openshift-adp/
total 0
drwxr-xr-x. 2 nobody nobody 6 Oct 8 00:17 .
drwxr-xr-x. 3 nobody nobody 27 Oct 8 00:17 ..
spampatt@spampatt-mac oadp-operator [secret-read-only]$ oc exec -it velero-5689985869-vn6b6 -- /bin/sh
Defaulted container "velero" out of: velero, openshift-velero-plugin (init), velero-plugin-for-aws (init)
sh-5.1$ ls -la credentials/cloud
ls: cannot access 'credentials/cloud': No such file or directory
sh-5.1$ ls -la tmp/credentials/openshift-adp/
total 4
drwxr-xr-x. 2 nobody nobody 37 Oct 8 00:21 .
drwxr-xr-x. 3 nobody nobody 27 Oct 8 00:21 ..
-rw-r--r--. 1 nobody nobody 116 Oct 8 00:21 cloud-credentials-cloud
sh-5.1$
spampatt@spampatt-mac oadp-operator [secret-read-only]$ oc exec -it velero-7758ddd869-5fr8m -- /bin/sh
Defaulted container "velero" out of: velero, openshift-velero-plugin (init), velero-plugin-for-aws (init)
sh-5.1$ ls -la credentials/
..2024_10_08_00_27_11.1709205179/ ..data/ cloud
sh-5.1$ ls -la credentials/cloud
lrwxrwxrwx. 1 root root 12 Oct 8 00:27 credentials/cloud -> ..data/cloud
sh-5.1$ ls -lalL credentials/cloud
-r--r-----. 1 root root 116 Oct 8 00:27 credentials/cloud
sh-5.1$ ls -lalL tmp/credentials/openshift-adp/
total 4
drwxr-xr-x. 2 nobody nobody 37 Oct 8 00:27 .
drwxr-xr-x. 3 nobody nobody 27 Oct 8 00:27 ..
-rw-r--r--. 1 nobody nobody 116 Oct 8 00:29 cloud-credentials-cloud
Matches the results in PR description for the 3 test scenarios.
/unhold discussed in scrum
/retest required
@weshayutin: The /retest
command does not accept any targets.
The following commands are available to trigger required jobs:
/test 4.14-ci-index
/test 4.14-e2e-test-aws
/test 4.14-e2e-test-kubevirt-aws
/test 4.14-images
/test 4.15-ci-index
/test 4.15-e2e-test-aws
/test 4.15-e2e-test-kubevirt-aws
/test 4.15-images
/test 4.16-ci-index
/test 4.16-e2e-test-aws
/test 4.16-e2e-test-kubevirt-aws
/test 4.16-images
/test 4.17-ci-index
/test 4.17-e2e-test-aws
/test 4.17-e2e-test-kubevirt-aws
/test 4.17-images
/test 4.18-dev-preview-ci-index
/test 4.18-dev-preview-images
/test images
/test unit-test
The following commands are available to trigger optional jobs:
/test 4.18-dev-preview-e2e-test-aws
Use /test all
to run all jobs.
/retest
/test ci/prow/4.17-images
/test 4.17-images
/test 4.17-ci-index
@kaovilai: The specified target(s) for /test
were not found.
The following commands are available to trigger required jobs:
/test 4.14-ci-index
/test 4.14-e2e-test-aws
/test 4.14-e2e-test-kubevirt-aws
/test 4.14-images
/test 4.15-ci-index
/test 4.15-e2e-test-aws
/test 4.15-e2e-test-kubevirt-aws
/test 4.15-images
/test 4.16-ci-index
/test 4.16-e2e-test-aws
/test 4.16-e2e-test-kubevirt-aws
/test 4.16-images
/test 4.17-ci-index
/test 4.17-e2e-test-aws
/test 4.17-e2e-test-kubevirt-aws
/test 4.17-images
/test 4.18-dev-preview-ci-index
/test 4.18-dev-preview-images
/test images
/test unit-test
The following commands are available to trigger optional jobs:
/test 4.18-dev-preview-e2e-test-aws
Use /test all
to run all jobs.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: kaovilai, mateusoliveira43, shubham-pampattiwar, sseago
The full list of commands accepted by this bot can be found here.
The pull request process is described here
/retest-required
Remaining retests: 0 against base HEAD c55320d9b775e6da3d14854af50824f309717b0d and 2 for PR HEAD 3000cb81e7983c61b45d81554aaab9e5b1ec5913 in total
@mateusoliveira43: The following test failed, say /retest
to rerun all failed tests or /retest-required
to rerun all mandatory failed tests:
Test name | Commit | Details | Required | Rerun command |
---|---|---|---|---|
ci/prow/4.18-dev-preview-e2e-test-aws | 3000cb81e7983c61b45d81554aaab9e5b1ec5913 | link | false | /test 4.18-dev-preview-e2e-test-aws |
Full PR test history. Your PR dashboard.
/cherrypick oadp-1.3
@mateusoliveira43: #1539 failed to apply on top of branch "oadp-1.3":
Applying: fix: Default permission for Secrets
Using index info to reconstruct a base tree...
M controllers/common.go
M controllers/velero.go
M controllers/velero_test.go
M pkg/common/common.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/common/common.go
Auto-merging controllers/velero_test.go
CONFLICT (content): Merge conflict in controllers/velero_test.go
Auto-merging controllers/velero.go
CONFLICT (content): Merge conflict in controllers/velero.go
Auto-merging controllers/common.go
CONFLICT (content): Merge conflict in controllers/common.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 fix: Default permission for Secrets
/cherrypick oadp-1.4
@mateusoliveira43: #1539 failed to apply on top of branch "oadp-1.4":
Applying: fix: Default permission for Secrets
Using index info to reconstruct a base tree...
M controllers/common.go
M controllers/velero.go
M controllers/velero_test.go
M pkg/common/common.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/common/common.go
Auto-merging controllers/velero_test.go
Auto-merging controllers/velero.go
CONFLICT (content): Merge conflict in controllers/velero.go
Auto-merging controllers/common.go
CONFLICT (content): Merge conflict in controllers/common.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 fix: Default permission for Secrets
Why the changes were made
have more restrictive permissions for Pods'
volume.secrets
.How to test the changes made
Run
make deploy-olm
and check if nothing was broken by the change.To test
volume.secrets
, create a DPA WITHOUT specifying credentials in BSL/VSL. Reference https://redhat-internal.slack.com/archives/C039LRSDC8Z/p1728048939142289In master, you should have this result
In this PR branch, you should have this result
You also need to test operations, like backup/restores, with these changes, since E2E tests do not create DPAs WITHOUT specifying credentials in BSL/VSL.
Test 1: No BSL credential and no VSL
DPA
Test 2: BSL credential and no VSL
DPA
Test 3: BSL credential and no VSL credential
DPA