Skip to content
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

Adds RBAC error during apply end-to-end test case #1425

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

seans3
Copy link
Contributor

@seans3 seans3 commented Feb 7, 2021

  • Adds test case in end-to-end test when an RBAC error happens during apply.
  • Validates the case is reported as failed and the object does not end up in the inventory.

@Liujingfang1
Copy link
Contributor

/lgtm

assertContains "namespace/test created"
assertContains "rolebinding.rbac.authorization.k8s.io/admin created"
assertContains "serviceaccount/user created"
wait 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what this wait is doing. Can you add a comment here to make it easier to understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may not need the wait, but it helps insure there is sufficient time for the resources to actually be spun up on the cluster (e.g namespace/test) after they've been created in the API Server. The best way to do this is to do a kubectl wait for=<CONDITION>, but this is a shortcut. Checkout the bash code for assertPodExists and assertPodNotExists which uses kubectl wait ....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest kubectl wait depending on the use case, but I think you may be intending to sleep 2 instead of wait 2 here. wait is going to delay until process id 2 exits. I'm not sure what pid 2 refers to in this case or if that's what was intended? Maybe I'm misunderstanding?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wait is a bash function defined earlier in the script which calls sleep. The number is the number of seconds to sleep.

kubectl config set-credentials user --token="$(kubectl get secrets -ojsonpath='{.data.token}' \
"$(kubectl get sa user -ojsonpath='{.secrets[0].name}')" \
| base64 -d)" > $OUTPUT_DIR/status
kubectl config set-context kind-kind:user --cluster=kind-kind --user=user > $OUTPUT_DIR/status
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can drop > $OUTPUT_DIR/status from these lines. Each of these commands will overwrite the contents of that output anyways so if we're not reading the contents the output shouldn't need to be saved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason for the > $OUTPUT_DIR/status: it keeps the output from being polluted. We'd rather just have . or E than the entire command output, which is what would happen without the redirection. This could probably be changed with a verbose flag, but I haven't created it yet.

e2e/live/end-to-end-test.sh Outdated Show resolved Hide resolved
@seans3
Copy link
Contributor Author

seans3 commented Feb 8, 2021

Output for this test case:

Testing RBAC error during apply
kpt live apply e2e/live/testdata/rbac-error-step-1
kpt live apply e2e/live/testdata/rbac-error-step-2
.........SUCCESS

@seans3 seans3 merged commit 3078d5e into kptdev:master Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/live customer deep engagement size/S 1 day
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants