Learning Paths Automation #5904

kkeirstead · 2024-01-29T18:20:08Z

Summary

This PR adds new workflows to aid in keeping learning paths up-to-date. This is a simplified version of what I previously demoed (I removed the Issue step in the middle now that we're auto-applying the suggestions), with the following flow:

Once a week (or on a different interval), "Check Learning Path Links" kicks off. This workflow compares the current state of the codebase with the previous one, as determined by a placeholder file "learning-path-sha.txt".
Upon completion, "Submit Learning Path PR" kicks off. This creates a PR that includes details about modified files, things to manually review, automatic suggestions, and files that are 'out-of-sync' (i.e. not tied to the correct SHA). The commit hashes within the learning path files are automatically updated, and the suggestions are automatically applied (via line-number changes to links).
This PR needs to be manually reviewed by whoever is handling the release and dependabot updates - outside of large refactors, this should be as simple as clicking on the PR's hyperlinks to ensure everything looks correct (and manually changing any of the links that need manual review due to more drastic changes).

There are a number of ways this could be improved (such as stable tags instead of commit hashes or increasing what this can handle automatically), but I'm trying not to over-engineer V1 of this so we can start using it here and in other repos. If we notice pain points, updates can be made to address them, but hopefully this should reduce the burden of keeping the learning paths up to date. By not running on every PR, this should hopefully alleviate some of the concerns from the first prototype about this getting in the way for every commit.

This is an example run on my fork - this simulates miscellaneous changes being made over the course of a week, and what PR would be auto-generated. The auto-applied suggestions handle in-file adjustments, and files that have ambiguous links or were renamed/deleted are listed under manually review. The "Out Of Sync" section shows that configuration.md has a link referencing main instead of the correct hash.

Release Notes Entry

* Create check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update ActivityExtensions.cs * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update submit-linter-suggestions.yml * Update submit-linter-suggestions.yml * Create submit-learning-paths-staleness-check.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update check-learning-path-links.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update check-learning-path-links.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update check-learning-path-links.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update check-learning-path-links.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update submit-learning-paths-staleness-check.yml * Update TestAssemblies.cs * Update CollectionRulePipeline.cs * Update submit-learning-paths-staleness-check.yml * Update CollectionRulePipeline.cs * Update CollectionRulePipeline.cs * Update submit-learning-paths-staleness-check.yml * Update ActivityExtensions.cs * Update CollectionRulePipeline.cs * Update TestAssemblies.cs

…ons/apply-learning-path-updates/action.yml

…s/apply-learning-path-updates/index.js

…essActions Kkeirstead/learning path staleness actions

schmittjoseph · 2024-01-29T19:22:04Z

.github/actions/apply-learning-path-updates/index.js

+
+function ReplaceOldWithNewText(content, oldText, newText)
+{
+  return content.replace(new RegExp(oldText, 'g'), newText);


We should avoid creating a regex expression from an arbitrary variable here. content.replaceAll(oldText, newText) should work here instead

schmittjoseph · 2024-01-29T19:26:38Z

.github/workflows/check-learning-path-links.yml

+        working-directory: ./head
+        run: |
+          mkdir -p ./learning-path-review
+          echo -n "${{ steps.check-links.outputs.modifiedFiles }}" > ./learning-path-review/modifiedFiles


Using substitution expressions here is a script injection risk. All of these should be passed in via an env block instead to avoid the risk.

Example:

dotnet-monitor/.github/workflows/update-release-version.yml

Lines 89 to 96 in 341ff64

if [ "$release_type" == "rtm" ]; then

sed -i "/<PreReleaseVersionLabel>.*/a\ \ \ \ <DotnetFinalVersionKind>release<\/DotnetFinalVersionKind>" $versionFile

fi

echo "release_version_file=$versionFile" >> $GITHUB_ENV

env:

release_type: ${{ inputs.release_type }}

release_version: ${{ inputs.release_version }}

schmittjoseph · 2024-01-29T19:28:49Z

.github/workflows/check-learning-path-links.yml

+name: 'Check Learning Path Links'
+on:
+  schedule: # Run once a week
+    - cron: '0 0 * * 1'


I'd be in favor of this running every other week or once a month. As it stands every week is a bit of noise for a manual review PR since there will always be changes everytime the HEAD commit is updated.

schmittjoseph · 2024-01-29T19:31:32Z

.github/workflows/check-learning-path-links.yml

+      - name: Checkout head
+        uses: actions/checkout@v4
+        with:
+          persist-credentials: true


What's the reason for persisted credentials? This is a security risk, especially since the token has elevated permissions and this step is ran first. Depending on the reason we can refactor this a couple different ways to minimize the risk.

schmittjoseph · 2024-01-29T19:32:44Z

.github/workflows/submit-learning-path-pr.yml

+          manuallyReview=${manuallyReview//,/<br />}
+          manuallyReviewSection="<h2>Manually Review:</h2>${manuallyReview}<br />"
+          if test -z "$manuallyReview"; then
+          manuallyReviewSection=""


nit: formatting, let's fix the indentation on the conditionals to make it easier to read.

schmittjoseph · 2024-01-29T19:39:11Z

.github/workflows/submit-learning-path-pr.yml

+          manuallyReview=$(cat ./learning-path-review/manuallyReview)
+          manuallyReview=${manuallyReview//,/<br />}
+          manuallyReviewSection="<h2>Manually Review:</h2>${manuallyReview}<br />"
+          if test -z "$manuallyReview"; then


Let's switch all of these if statements to use the bracket commands instead of explicit test as that's more conventional in bash scripts.

e.g. if [ -z "$manuallyReview" ]

schmittjoseph

Blocking on security related comments

schmittjoseph · 2024-01-29T19:50:29Z

.github/workflows/submit-learning-path-pr.yml

@@ -0,0 +1,117 @@
+name: 'Submit Learning Path PR'


I'm not sure I see the value of having split workflows here. It's adding a lot of complexity (and potential security risks) without adding value now that the proposed issue step has been removed.

Let's consolidate the split workflows and actions into a single one instead. If we consolidate them, then the action can simply generate a report (pr body) instead of outputting several variables and having a bash script do it which is more prone to script injections.

kkeirstead · 2024-01-31T15:42:00Z

Closing this to prevent churn while testing/debugging new changes...I made the mistake of merging directly from my fork's main branch where I had been running the workflow. Will make sure comments are not lost.

kkeirstead added 30 commits October 2, 2023 10:29

Update architecture.md

0d45cbd

Update collectionrules.md

67cf509

Update collectionrules.md

1673989

Update check-learning-path-links.yml

5b5de00

Update check-learning-path-links.yml

9ce6f72

Update check-learning-path-links.yml

0598e8a

Update submit-linter-suggestions.yml

88ac574

Update check-learning-path-links.yml

e7d6ae7

Update submit-learning-paths-staleness-check.yml

8fba8a1

Update submit-learning-paths-staleness-check.yml

c5895d0

Update submit-learning-paths-staleness-check.yml

7c7530d

Update submit-learning-paths-staleness-check.yml

7c8f459

Update submit-learning-paths-staleness-check.yml

bf6c353

Update submit-learning-paths-staleness-check.yml

7d7aa67

Update submit-learning-paths-staleness-check.yml

8bb85e2

Update submit-learning-paths-staleness-check.yml

944ea6d

Update submit-learning-paths-staleness-check.yml

a960d51

Update submit-learning-paths-staleness-check.yml

506152d

Update submit-linter-suggestions.yml

84604ad

Update check-learning-path-links.yml

c360b09

Update check-learning-path-links.yml

62d84cb

Update configuration.md

82c54b4

Update collectionrules.md

0a1a0f7

Update testing.md

b287d1b

Update check-learning-path-links.yml

0d1afa3

Create check-learning-path-links.yml

cee5e3e

Create submit-learning-paths-staleness-check.yml

16b85eb

Update CollectionRulePipelineState.cs

9394604

Update collectionrules.md

68ba9e9

kkeirstead added 16 commits January 29, 2024 09:06

Rename index.js to index.js

952f10c

Create index.js

1a348ba

Create action.yml

ea1dd10

Rename .github/apply-learning-path-updates/action.yml to .github/acti…

02341c2

…ons/apply-learning-path-updates/action.yml

Rename .github/apply-learning-path-updates/index.js to .github/action…

d427f0c

…s/apply-learning-path-updates/index.js

Update index.js

19b027a

Update index.js

6e08498

Update submit-learning-path-pr.yml

033256f

Update check-learning-path-links.yml

8cf3318

Merge pull request #335 from kkeirstead/kkeirstead/LearningPathStalen…

544d8c3

…essActions Kkeirstead/learning path staleness actions

Update ServiceCollectionExtensions.cs

f886104

Update ServiceCollectionExtensions.cs

7fae97a

Update api.md

42c9dc3

Update collectionrules.md

b8b605f

Update configuration.md

214cbf4

Update egress.md

d353d8b

kkeirstead requested a review from a team as a code owner January 29, 2024 18:20

dotnet-policy-service bot added the needs-review label Jan 29, 2024

schmittjoseph reviewed Jan 29, 2024

View reviewed changes

dotnet-policy-service bot removed the needs-review label Jan 29, 2024

schmittjoseph reviewed Jan 29, 2024

View reviewed changes

schmittjoseph requested changes Jan 29, 2024

View reviewed changes

schmittjoseph reviewed Jan 29, 2024

View reviewed changes

kkeirstead closed this Jan 31, 2024

kkeirstead mentioned this pull request Jan 31, 2024

Learning Paths Automation #5940

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Learning Paths Automation #5904

Learning Paths Automation #5904

kkeirstead commented Jan 29, 2024

schmittjoseph Jan 29, 2024 •

edited

Loading

schmittjoseph Jan 29, 2024 •

edited

Loading

schmittjoseph Jan 29, 2024 •

edited

Loading

schmittjoseph Jan 29, 2024

schmittjoseph Jan 29, 2024

schmittjoseph Jan 29, 2024

schmittjoseph left a comment

schmittjoseph Jan 29, 2024 •

edited

Loading

kkeirstead commented Jan 31, 2024

	if [ "$release_type" == "rtm" ]; then
	sed -i "/<PreReleaseVersionLabel>.*/a\ \ \ \ <DotnetFinalVersionKind>release<\/DotnetFinalVersionKind>" $versionFile
	fi

	echo "release_version_file=$versionFile" >> $GITHUB_ENV
	env:
	release_type: ${{ inputs.release_type }}
	release_version: ${{ inputs.release_version }}

Learning Paths Automation #5904

Learning Paths Automation #5904

Conversation

kkeirstead commented Jan 29, 2024

Summary

Release Notes Entry

schmittjoseph Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

schmittjoseph Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

schmittjoseph Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

schmittjoseph Jan 29, 2024

Choose a reason for hiding this comment

schmittjoseph Jan 29, 2024

Choose a reason for hiding this comment

schmittjoseph Jan 29, 2024

Choose a reason for hiding this comment

schmittjoseph left a comment

Choose a reason for hiding this comment

schmittjoseph Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

kkeirstead commented Jan 31, 2024

schmittjoseph Jan 29, 2024 •

edited

Loading

schmittjoseph Jan 29, 2024 •

edited

Loading

schmittjoseph Jan 29, 2024 •

edited

Loading

schmittjoseph Jan 29, 2024 •

edited

Loading