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

fix(query): azure aks rbac-variable changed #5652

Merged
merged 3 commits into from
Aug 1, 2022
Merged

fix(query): azure aks rbac-variable changed #5652

merged 3 commits into from
Aug 1, 2022

Conversation

rndmh3ro
Copy link
Contributor

@rndmh3ro rndmh3ro commented Jul 29, 2022

Proposed Changes

I submit this contribution under the Apache-2.0 license.

@rndmh3ro
Copy link
Contributor Author

The failing test does not seem to have to do with my changes..

@rafaela-soares rafaela-soares added the community Community contribution label Jul 29, 2022
@rafaela-soares
Copy link
Contributor

The failing test does not seem to have to do with my changes..

Yes, you are right, @rndmh3ro! Do you mind fixing the samples, please?

@rndmh3ro
Copy link
Contributor Author

rndmh3ro commented Aug 1, 2022

The failing test does not seem to have to do with my changes..

Yes, you are right, @rndmh3ro! Do you mind fixing the samples, please?

Locally there are really much errors (>50) so I'd rather not fix them in this PR. :D

@rafaela-soares
Copy link
Contributor

The failing test does not seem to have to do with my changes..

Yes, you are right, @rndmh3ro! Do you mind fixing the samples, please?

Locally there are really much errors (>50) so I'd rather not fix them in this PR. :D

We can only approve and merge the PR if all the tests passed. I will talk to the team to take a look and solve the validate-terraform-samples errors. After the errors are fixed in the master, you can merge your branch with the master 😊

@rndmh3ro
Copy link
Contributor Author

rndmh3ro commented Aug 1, 2022

We can only approve and merge the PR if all the tests passed. I will talk to the team to take a look and solve the validate-terraform-samples errors. After the errors are fixed in the master, you can merge your branch with the master blush

When running the same python-script which github actions runs, I get far less errors, so I fixed these. :)

@rndmh3ro
Copy link
Contributor Author

rndmh3ro commented Aug 1, 2022

I don't know why the tests fail now. :(

@rafaela-soares
Copy link
Contributor

I don't know why the tests fail now. :(

Thank you so much for fixing validate-terraform-samples errors!

The test are failing now because of this change:

image

You will need to change the result line in assets\queries\common\passwords_and_secrets\test\positive_expected_result.json

@rafaela-soares
Copy link
Contributor

I don't know why the tests fail now. :(

Thank you so much for fixing validate-terraform-samples errors!

The test are failing now because of this change:

image

You will need to change the result line in assets\queries\common\passwords_and_secrets\test\positive_expected_result.json

You will need to do this change, @rndmh3ro :

image

Copy link
Contributor

@rafaela-soares rafaela-soares left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Thank you so much, @rndmh3ro!

@rafaela-soares rafaela-soares merged commit 4e807e7 into Checkmarx:master Aug 1, 2022
@rndmh3ro rndmh3ro deleted the aks_rbac branch August 1, 2022 13:56
@rafaela-soares rafaela-soares added the query New query feature label Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution query New query feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants