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

DevSecOps pipeline with GitHub Actions #760

Merged
merged 239 commits into from
Mar 24, 2021

Conversation

kulov
Copy link
Contributor

@kulov kulov commented Jan 22, 2021

Resolves #750

DevSecOps pipeline with GitHub Actions to increase project confidence and maturity level.
This would increase project adoption to professional developers and enterprises who are much more willing to use secure and reliable OSS components but afraid of meeting quality and security bars.

PR Type

This PR adds new improved GitHub workflow to enable static code analysis, automatic releases and code signing using pfx file stored in GH Secrets.
This PR also adds GitHub Dependabot checks, runs CodeQL analysis and SonarCloud static source analysis.
This PR also provides all required documentation to configure all implemented tools and results with related screenshots.

Other information

Work still in progress.
This is no obligation, no strings attached work delivered to you and built with ❤ by Pipeline Foundation.
Any feedback will be taken in detailed consideration.
Support of final delivery will be provided for one year after PR approval.

aleks-ivanov and others added 30 commits January 12, 2021 14:35
* Add SonarCloud and Testspace.
Add Manual Run entry to documentation AB#198
Add Code Scanning alerts bulk dismissal workflow AB#196 AB#203 AB#209 AB#210
Add initial entry for the release sequence to the documentation AB#202
duplicate of CI-CD Documentation.md
@kulov
Copy link
Contributor Author

kulov commented Mar 11, 2021

@Jasonstein, would you like to set the values of Package.StoreAssociation.xml using GitHub secrets?
We would be happy to do so if you provide us with the xml file.

@0x7c13
Copy link
Owner

0x7c13 commented Mar 19, 2021

@Jasonstein, would you like to set the values of Package.StoreAssociation.xml using GitHub secrets?
We would be happy to do so if you provide us with the xml file.

I am fine with that, but what about putting the AppCenter's secret token in the Github secrets? How it can be applied to the code since it is actually a code change whenever I build the release package?

Lines="$([System.IO.File]::ReadAllText($(InputFile)).Replace('AppCenterSecret = null','AppCenterSecret = "$(AppCenterSecret)"'))"
Overwrite="true"
Encoding="Unicode"/>
</Target>
Copy link
Contributor

Choose a reason for hiding this comment

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

@Jasonstein This target will handle replacing AppCenterSecret with actual value. The actual value is passed on to MSBuild as an environment variable with the same name from Github secrets.

Copy link
Owner

Choose a reason for hiding this comment

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

Awesome! I will add a comment in the code as well.

/p:AppxPackageDir=$env:ARTIFACTS_DIR `
/p:PackageCertificateKeyFile=$env:PACKAGE_CERTIFICATE_KEYFILE `
/p:PackageCertificatePassword=$env:PACKAGE_CERTIFICATE_PASSWORD `
/p:AppCenterSecret=$env:APP_CENTER_SECRET
Copy link
Contributor

Choose a reason for hiding this comment

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

@Jasonstein you just have to store the AppCenterSecret as APP_CENTER_SECRET in Github secrets.

@soumyamahunt
Copy link
Contributor

@Jasonstein Also store upload task could be automated, @kulov are you interested in providing this??

@0x7c13
Copy link
Owner

0x7c13 commented Mar 19, 2021

@Jasonstein Also store upload task could be automated, @kulov are you interested in providing this??

@kulov PM me an email address and I will send you the Package.StoreAssociation.xml file.

@kulov
Copy link
Contributor Author

kulov commented Mar 20, 2021

@soumyamahunt Absolutely!

@kulov
Copy link
Contributor Author

kulov commented Mar 20, 2021

@Jasonstein, I've just sent you email.
Should it be Package.StoreAssociation.xml gets added to the PR after removing all secrets?

@soumyamahunt
Copy link
Contributor

@Jasonstein Is there any security concern of keeping Package.StoreAssociation.xml in the repo itself?? Most UWP/MSIX repos keep this file in the repo itself.

@0x7c13
Copy link
Owner

0x7c13 commented Mar 20, 2021

@Jasonstein Is there any security concern of keeping Package.StoreAssociation.xml in the repo itself?? Most UWP/MSIX repos keep this file in the repo itself.

I am actually thinking of it. There is no harm since there is no actual secret or certificate-related information stored as part of the store association.xml anyway.

@@ -69,6 +69,17 @@
</ItemGroup>
</Target>

<Target Name="AssignAppCenterSecret" BeforeTargets="BeforeBuild" Condition=" $(AppCenterSecret) != '' ">
Copy link
Contributor

Choose a reason for hiding this comment

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

@aleks-ivanov Don't put this target in Package.targets, put it in the project file instead. The reason I created this separate file for targets is because of #523 and these common targets will be both imported by UWP project and packaging project. Since the AssignAppCenterSecret target is project specific it doesn't make any sense to put it in a separate file.

Copy link
Contributor

@aleks-ivanov aleks-ivanov Mar 22, 2021

Choose a reason for hiding this comment

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

@soumyamahunt understood, I will move it back to the project file with the next feature merge 🙂

aleks-ivanov and others added 2 commits March 23, 2021 13:25
* add publish-to-store feature, add necessary files and changes and move AppCenterSecret to project file

* update version in appxmanifest

* add publish-to-store automation and documentation for it
@aleks-ivanov
Copy link
Contributor

aleks-ivanov commented Mar 24, 2021

@soumyamahunt @Jasonstein

Please review the latest changes and let me know if everything is done correctly in terms of the project, manifest and association files, and if there is anything I've missed 🙂

I've thoroughly tested the automated submission creation in the Microsoft Partner Center and with the proper setup it works without a hitch. In the CI-CD_DOCUMENTATION.md under entry 6 (line 268), I've written the setup instructions. There are a few prerequisites, but they are all quite straight forward. Either way, let me know if you need any further assistance with the setup.


- name: Publish to Windows Store
id: publish_to_store
uses: isaacrlevin/windows-store-action@main
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with isaacrlevin/[email protected].

@soumyamahunt
Copy link
Contributor

@aleks-ivanov @kulov is there any reason for separate codeql-analysis action, can't it just be included in ci job??

@Jasonstein the previous build action can be removed in favor of the main action, any thoughts??

@aleks-ivanov
Copy link
Contributor

aleks-ivanov commented Mar 24, 2021

@soumyamahunt my reasoning for the separation is to allow the CodeQL analysis to run on a CRON schedule of - cron: '0 8 * * *, so that the project can make use of the constantly evolving CodeQL query database.

Also since the main pipeline doesn't require execution on that trigger, it is more elegant to separate them, than to pile on unnecessary logic.

@0x7c13
Copy link
Owner

0x7c13 commented Mar 24, 2021

@aleks-ivanov @kulov is there any reason for separate codeql-analysis action, can't it just be included in ci job??

@Jasonstein the previous build action can be removed in favor of the main action, any thoughts??

Sure, we can remove the build.
Another question for you guys: what's the story of the versioning here based on the current CI/CD approach. Should we bump the version before triggering the CD pipeline for release or it can auto increment? @aleks-ivanov

@aleks-ivanov
Copy link
Contributor

@Jasonstein

The automatic incrementing is certainly possible with the following changes to the pipeline use:

  • the commit message would determine if you want to bump the version or not
  • each commit that you wish to bump the version of the project, should be accompanied by a label corresponding to one of the version numbers (major, minor or patch):

git commit -m "fix: update language resource for Spanish" - this commit will create a new GitHub tag with bumped version from let's say if the latest GitHub tag is 1.0.0 to 1.0.1 automatically (the other two labels are feat for minor and perf for major with an additional footer tag of BREAKING CHANGE:)

  • consequently, that number can be used in any part of the pipeline e.g. to automatically bump the version in the Package.appxmanifest file, so that the build process executes with the new version

If that sounds good, we can create a separate PR with the change to automatic versioning, so you can compare the two implementations and see which one is more suitable to you ? 🙂


For further details, this is how the process works currently:

  1. in the CI job of the pipeline, the version is retrieved from Identity.Version in the Package.appxmanifest file
  2. then the latest tag's value is retrieved from the project's remote GitHub repo's tag list
  3. based on those two values, several processes are evaluated for execution:
  • if the version is different from the tag AND the trigger of the pipeline is a push commit to the default branch -> a new tag is pushed to GitHub
  • if the version is different from the tag AND the trigger of the pipeline is a push commit to the default branch -> the whole CD job is executed, which includes the creation of the GitHub release and the submission to the Microsoft Partner Center (Windows Store)

So if the Identity.Version in the Package.appxmanifest is left the same as before (you don't manually bump it), those processes would be skipped.

@0x7c13
Copy link
Owner

0x7c13 commented Mar 24, 2021

@Jasonstein

The automatic incrementing is certainly possible with the following changes to the pipeline use:

  • the commit message would determine if you want to bump the version or not
  • each commit that you wish to bump the version of the project, should be accompanied by a label corresponding to one of the version numbers (major, minor or patch):

git commit -m "fix: update language resource for Spanish" - this commit will create a new GitHub tag with bumped version from let's say if the latest GitHub tag is 1.0.0 to 1.0.1 automatically (the other two labels are feat for minor and perf for major with an additional footer tag of BREAKING CHANGE:)

  • consequently, that number can be used in any part of the pipeline e.g. to automatically bump the version in the Package.appxmanifest file, so that the build process executes with the new version

If that sounds good, we can create a separate PR with the change to automatic versioning, so you can compare the two implementations and see which one is more suitable to you ? 🙂

For further details, this is how the process works currently:

  1. in the CI job of the pipeline, the version is retrieved from Identity.Version in the Package.appxmanifest file
  2. then the latest tag's value is retrieved from the project's remote GitHub repo's tag list
  3. based on those two values, several processes are evaluated for execution:
  • if the version is different from the tag AND the trigger of the pipeline is a push commit to the default branch -> a new tag is pushed to GitHub
  • if the version is different from the tag AND the trigger of the pipeline is a push commit to the default branch -> the whole CD job is executed, which includes the creation of the GitHub release and the submission to the Microsoft Partner Center (Windows Store)

So if the Identity.Version in the Package.appxmanifest is left the same as before (you don't manually bump it), those processes would be skipped.

Thanks, makes total sense. We can do it in a separate PR. Btw, really appreciate your work here for improving the CI/CD pipeline for Notepads and this looks awesome. @kulov @aleks-ivanov @soumyamahunt

@0x7c13 0x7c13 merged commit 38dfba1 into 0x7c13:master Mar 24, 2021
@aleks-ivanov aleks-ivanov deleted the devsecops branch March 25, 2021 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] Implement GitHub DevSecOps workflow.
5 participants