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

ci: add psrule validation in both github and ADO #7323

Closed
wants to merge 10 commits into from

Conversation

juju4
Copy link
Contributor

@juju4 juju4 commented Feb 11, 2023

Change(s):

Reason for Change(s):

  • Ensure ARM templates best practices are applied

Version Updated:

  • N/A

Testing Completed:

  • Yes, Github Actions

Checked that the validations are passing and have addressed any issues that are present:

  • ok

Example output:
https://github.com/juju4/Azure-Sentinel/actions/runs/4152879216/jobs/7184135632#step:3:37

@juju4 juju4 requested review from a team as code owners February 11, 2023 20:04
@juju4 juju4 mentioned this pull request Feb 11, 2023
@v-sabiraj v-sabiraj added the Validation Validation specialty review needed label Feb 12, 2023
@v-atulyadav
Copy link
Contributor

Hi @rushriva, please provide your feedback.Thanks

@v-atulyadav
Copy link
Contributor

Hi @rushriva, can you please provide your feedback on this. Thanks

@v-atulyadav
Copy link
Contributor

Hi @rushriva, please provide your feedback.Thanks

@v-atulyadav
Copy link
Contributor

Hi @rushriva, can you please provide your feedback on this. Thanks

@v-atulyadav
Copy link
Contributor

Hi @rushriva, could you please provide your feedback on this. Thanks

@v-atulyadav
Copy link
Contributor

Hi @rushriva, can you please provide your feedback on this. Thanks

@v-atulyadav
Copy link
Contributor

v-atulyadav commented Mar 10, 2023

Hi @rushriva, could you please provide your feedback. Thanks

Hi @juju4, please update your branch from master and push the branch again. Thanks

@juju4
Copy link
Contributor Author

juju4 commented Mar 10, 2023

current master merged

@v-atulyadav
Copy link
Contributor

Hi @vakohl, please provide your feedback on this request. Thanks

@v-atulyadav
Copy link
Contributor

Hi @vakohl, could you please provide your feedback on this request. Thanks

@v-atulyadav
Copy link
Contributor

Hi @vakohl, please provide your feedback on this request. Thanks

@v-atulyadav
Copy link
Contributor

Hi @vakohl,
could you please provide your feedback on this request.
Thanks

@v-atulyadav
Copy link
Contributor

Hi @vakohl,
please provide your feedback on this request.
Thanks

@v-atulyadav
Copy link
Contributor

Hi @vakohl,
could you please provide your feedback on this request.
Thanks

@v-atulyadav
Copy link
Contributor

Hi @vakohl,
please provide your feedback on this request.
Thanks

@v-atulyadav
Copy link
Contributor

Hi @vakohl,
could you please provide your feedback on this request.
Thanks

@v-atulyadav
Copy link
Contributor

Hi @daxesht, could you please provide your feedback on this. Thanks

@v-atulyadav
Copy link
Contributor

Hi @daxesht, please provide your feedback on this. Thanks

@v-atulyadav
Copy link
Contributor

Hi @daxesht, could you please provide your feedback on this. Thanks

@v-atulyadav
Copy link
Contributor

Hi @daxesht, please provide your feedback on this. Thanks

@juju4
Copy link
Contributor Author

juju4 commented Jun 17, 2023

Any update on this or path forward?

@v-atulyadav
Copy link
Contributor

Hi @juju4, I apologize for the inconvenience. We will provide you with an update by the 13th. Please update your branch from master in the meantime. Thanks

@v-atulyadav
Copy link
Contributor

Hi @juju4, Having analyzed the contents, we are not able to determine the reason for this change. Could you please elaborate on this and provide screenshots of this functionality if possible? Also the changes you are requesting are for both Github workflows and Azure pipelines, can't we handle this in one place instead of both? It would also be great if we could connect over a call to discuss further. Thanks

@v-atulyadav
Copy link
Contributor

Hi @juju4, waiting for your response over above comments and also please resolve branch conflicts. Thanks

@v-atulyadav
Copy link
Contributor

Hi @juju4, please check comments above and act accordingly. Thanks

@v-atulyadav
Copy link
Contributor

Hi @juju4, I am looking forward to your response to the above comments, as well as the resolution of branch conflicts. Thanks

@v-atulyadav
Copy link
Contributor

Hi @juju4, please respond over above comments and also please resolve branch conflicts. Thanks

@v-atulyadav
Copy link
Contributor

Hi @juju4, please check comments above and act accordingly. Thanks

@v-atulyadav
Copy link
Contributor

Hi @juju4, hope you are doing well. Just wanted to check if you got a chance to look at the feedback shared. Please feel free to reach out to us for any queries and/or support. Thanks

@juju4
Copy link
Contributor Author

juju4 commented Aug 5, 2023

The content of the PR has been explained in #7323 (comment) aka "Lint validation of ARM templates with https://github.com/Azure/PSRule.Rules.Azure/".
It applies to both Github Action and ADO because ultimately it should be available to whatever backend is used as pipeline with Sentinel Repository feature.

It adds no functionality outside of validating more content but goes further than existing validations. similar to #7056

@v-atulyadav
Copy link
Contributor

Noted @juju4,
We will check internally and provide you with an update as soon as possible.
Thanks

@vakohl
Copy link
Contributor

vakohl commented Aug 7, 2023

The content of the PR has been explained in #7323 (comment) aka "Lint validation of ARM templates with https://github.com/Azure/PSRule.Rules.Azure/". It applies to both Github Action and ADO because ultimately it should be available to whatever backend is used as pipeline with Sentinel Repository feature.

It adds no functionality outside of validating more content but goes further than existing validations. similar to #7056

Hi @mkchiliveri , Can you please have a look? Seems related to our build pipeline

@vakohl vakohl assigned mkchiliveri and unassigned vakohl Aug 7, 2023
@v-atulyadav
Copy link
Contributor

Hi @juju4,
We will provide you with an update on this PR by the middle of next week, and if we find any difficulty, we will reach out to you.
Thanks

@mkchiliveri
Copy link
Contributor

Hi @juju4,

As discussed, can you please check on if we can enforce this rule only on files committed as part of PR so that author can take an action immediately instead of the full review.

@juju4
Copy link
Contributor Author

juju4 commented Sep 2, 2023

Per Azure/ResourceModules#2150, not possible directly either.
Possible workaround mentioned:

  1. first retrieves changed files, then 2. applies token replacement, then 3. runs psrule only on diff retrieved by step 1

Within github actions, maybe like microsoft/PSRule#688 with dependency on tj-actions/changed-files
Within ADO, I don't see a way to do it except PR is single commit which is unlikely. That would be using git diff --name-only --relative --diff-filter AMCR HEAD^ HEAD . as said in https://stackoverflow.com/questions/72451778/azure-devops-pipeline-get-modified-or-added-files-only

@v-atulyadav
Copy link
Contributor

Hi @juju4, as discussed we will take this as internal feature and try to develop this in future. Thanks

@v-atulyadav v-atulyadav closed this Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Validation Validation specialty review needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants