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

Add MSGraph and SPO library max versions #908

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

schrolla
Copy link
Collaborator

@schrolla schrolla commented Feb 14, 2024

🗣 Description

The current ScubaGear depends on a number of Microsoft PowerShell modules to interact with M365 services. Previously, ScubaGear setup installed the latest version of these libraries available in PSGallery. This update sets the maximum version for the Microsoft.Graph.* modules to v2.12.0 and the Microsoft.Online.SharePoint.PowerShell to v16.0.24322.1200 so that newer versions will not be installed.

💭 Motivation and context

This change is needed due to errors in the upstream modules that causes ScubaGear to fail. The solution chosen was to "pin" the module versions used to the last working versions until the upstream modules have been fixed to prevent ScubaGear users from receiving these errors.

Closes #889
Closes #903
Closes #904

🧪 Testing

Changes were tested by verifying failures after removing all Microsoft modules and re-running Setup.ps1 from the latest ScubaGear main branch. Then, after updating the RequiredVersions.ps1 file to pin the max versions, removing all libraries again and re-running Setup.ps1. Confirmed that the max pinned versions of the modules were installed and ran Invoke-Scuba -p * with no errors.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • PR targets the correct parent branch (e.g., main or release-name) for merge.
  • Changes are limited to a single goal - eschew scope creep!
  • Changes are sized such that they do not touch excessive number of files.
  • All future TODOs are captured in issues, which are referenced in code comments.
  • These code changes follow the ScubaGear content style guide.
  • Related issues these changes resolve are linked preferably via closing keywords.
  • All relevant type-of-change labels added.
  • All relevant project fields are set.
  • All relevant repo and/or project documentation updated to reflect these changes.
  • Unit tests added/updated to cover PowerShell and Rego changes.
  • Functional tests added/updated to cover PowerShell and Rego changes.
  • All relevant functional tests passed.
  • All automated checks (e.g., linting, static analysis, unit/smoke tests) passed.

✅ Pre-merge checklist

  • PR passed smoke test check.

  • Feature branch has been rebased against changes from parent branch, as needed

    Use Rebase branch button below or use this reference to rebase from the command line.

  • Resolved all merge conflicts on branch

  • Notified merge coordinator that PR is ready for merge via comment mention

✅ Post-merge checklist

  • Feature branch deleted after merge to clean up repository.
  • Verified that all checks pass on parent branch (e.g., main or release-name) after merge.

@schrolla schrolla added the bug This issue or pull request addresses broken functionality label Feb 14, 2024
@schrolla schrolla added this to the Glacier milestone Feb 14, 2024
@schrolla schrolla self-assigned this Feb 14, 2024
@schrolla schrolla marked this pull request as ready for review February 14, 2024 21:48
@tkol2022
Copy link
Collaborator

Test Results

Looks like setup installed the expected "pinned" module versions on a virtual machine. I also tested on my laptop.
image

The only remaining comment that I have on this topic is that on my laptop, just running UninstallModules.ps1 one time was not enough to rectify the Sharepoint error. This is because that script only removes the latest version of the respective module. Since my machine was two versions ahead of the pinned Sharepoint version, I had to manually uninstall one more Sharepoint version after I ran UninstallModules. Once I did that my machine was at the expected state as in the screenshot below - and then Sharepoint ran successfully. I'm wondering how we can communicate this to users and provide them guidance.

image

@tkol2022 tkol2022 self-requested a review February 14, 2024 23:30
Copy link
Collaborator

@tkol2022 tkol2022 left a comment

Choose a reason for hiding this comment

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

Approved pending resolution of a comment that I left about the fact that getting the machine to a known working state is tricky even with this "pinned" version fix.

@schrolla
Copy link
Collaborator Author

schrolla commented Feb 15, 2024

@tkol2022 Concur that those users that need to downgrade are still in a tricky state. The fix here is meant to prevent more users from getting into that state in the first place more than address those that already are.

We could certainly log an issue to update the setup script to better account for pinning by checking that no versions outside of those allowed are installed, perhaps. But that could also cause some issues in some other cases. So I'd rather put that as a TODO outside the bugfix and handle any remaining downgrade users via instructions to remove ALL versions of the affected libraries via Uninstall-Module before running Setup.ps1 again.

Does that seem like a reasonable way forward?

Test Results

Looks like setup installed the expected "pinned" module versions on a virtual machine. I also tested on my laptop. image

The only remaining comment that I have on this topic is that on my laptop, just running UninstallModules.ps1 one time was not enough to rectify the Sharepoint error. This is because that script only removes the latest version of the respective module. Since my machine was two versions ahead of the pinned Sharepoint version, I had to manually uninstall one more Sharepoint version after I ran UninstallModules. Once I did that my machine was at the expected state as in the screenshot below - and then Sharepoint ran successfully. I'm wondering how we can communicate this to users and provide them guidance.

image

@tkol2022
Copy link
Collaborator

@tkol2022 Concur that those users that need to downgrade are still in a tricky state. The fix here is meant to prevent more users from getting into that state in the first place more than address those that already are.

We could certainly log an issue to update the setup script to better account for pinning by checking that no versions outside of those allowed are installed, perhaps. But that could also cause some issues in some other cases. So I'd rather put that as a TODO outside the bugfix and handle any remaining downgrade users via instructions to remove ALL versions of the affected libraries via Uninstall-Module before running Setup.ps1 again.

Does that seem like a reasonable way forward?

Test Results

Looks like setup installed the expected "pinned" module versions on a virtual machine. I also tested on my laptop. image
The only remaining comment that I have on this topic is that on my laptop, just running UninstallModules.ps1 one time was not enough to rectify the Sharepoint error. This is because that script only removes the latest version of the respective module. Since my machine was two versions ahead of the pinned Sharepoint version, I had to manually uninstall one more Sharepoint version after I ran UninstallModules. Once I did that my machine was at the expected state as in the screenshot below - and then Sharepoint ran successfully. I'm wondering how we can communicate this to users and provide them guidance.
image

Yes. Right now, I would monitor the Microsoft Github to see if any fixes are in the works. We might not need to do anything - highly likely that others have complained about the modules being broken and Microsoft is fixing it.

Copy link
Collaborator

@nanda-katikaneni nanda-katikaneni left a comment

Choose a reason for hiding this comment

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

Tested it on a new VM to confirm that with max versions set as in this branch, there are no issues with either aad or SharePoint. Approving.

* Pin Microsoft.Online.SharePoint.PowerShell to max version 16.0.24322.12000
@schrolla schrolla force-pushed the pin-graph-sharepoint-library-versions branch from 8d09372 to d98a9ba Compare February 15, 2024 16:29
@schrolla
Copy link
Collaborator Author

Yes. Right now, I would monitor the Microsoft Github to see if any fixes are in the works. We might not need to do anything - highly likely that others have complained about the modules being broken and Microsoft is fixing it.

Based on their previous release cadence of 2-4 weeks, I think that would have us in a wait and see mode for at least a week. We can release a hotfix version and watch for a fix to test for the SDK. If it is resolved we can decide if we want to back out the pinned versions prior to our next planned release or handle differently long term. Reasonable?

@nanda-katikaneni nanda-katikaneni merged commit 4f2d982 into main Feb 16, 2024
3 checks passed
@nanda-katikaneni nanda-katikaneni deleted the pin-graph-sharepoint-library-versions branch February 16, 2024 15:00
schrolla added a commit that referenced this pull request Feb 16, 2024
* Pin Microsoft.Online.SharePoint.PowerShell to max version 16.0.24322.12000
james-garriss pushed a commit that referenced this pull request Feb 19, 2024
* Pin Microsoft.Online.SharePoint.PowerShell to max version 16.0.24322.12000
nanda-katikaneni pushed a commit that referenced this pull request Feb 19, 2024
* initial commit

* add workflow_call

* add workflow_call to linter

* limit to this branch

* add lint failures

* add ps lint

* add markdown

* fix needs

* add ps unit test

* add opa unit tests

* add workflow_call to opa

* on run on dispatch

* put triggers back

* put paths on workflow

* put paths back after test

* use workflow_call alone

* use test files to avoid dupe workflows

* add not failure

* get parallel

* test w/o workflow files

* * Pin Microsoft.Graph.* max version to 2.12.0 (#908)

* Pin Microsoft.Online.SharePoint.PowerShell to max version 16.0.24322.12000

* remove workflow yaml file checks

* add added or modified

* add base head

* minor change to YAML

* set ref

* add doEverything

* fix input name

* remove or

* fix or

* fix or, open push

* update md files and workflow branch

* remove branch for uses

* Remove 1 and 2 from lint

---------

Co-authored-by: Addam Schroll <108814318+schrolla@users.noreply.github.com>
@radiandrew
Copy link

Just chiming in to report that I upgraded to version 16.0.24614.12000 of Microsoft.Online.SharePoint.PowerShell and the issue seems to be fixed. Cannot comment on the MSGraph dependency, but if that's also case there, it would be a good time to decide whether to keep the version pinned or not.

@tkol2022
Copy link
Collaborator

tkol2022 commented Mar 7, 2024

Just chiming in to report that I upgraded to version 16.0.24614.12000 of Microsoft.Online.SharePoint.PowerShell and the issue seems to be fixed. Cannot comment on the MSGraph dependency, but if that's also case there, it would be a good time to decide whether to keep the version pinned or not.

Thx a lot for your feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue or pull request addresses broken functionality
Projects
None yet
4 participants