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

[BUG] Set-PnPTenantCdnPolicy doesn't accept null policy values #3935

Closed
1 of 6 tasks
ricmestre opened this issue May 3, 2024 · 6 comments · Fixed by #3937
Closed
1 of 6 tasks

[BUG] Set-PnPTenantCdnPolicy doesn't accept null policy values #3935

ricmestre opened this issue May 3, 2024 · 6 comments · Fixed by #3937
Labels
bug Something isn't working

Comments

@ricmestre
Copy link

Reporting an Issue or Missing Feature

On a newly created tenant I was applying several policies through Microsoft365DSC and noticed that due to a bug there, which I still didn't report or created a PR for, it applied a Tenant CDN policy with ExcludeRestrictedSiteClassifications set to what I had previously applied on IncludeFileExtensions which of course is wrong, but to my surprise I couldn't set ExcludeRestrictedSiteClassifications back to empty with Set-PnPTenantCdnPolicy as it was the default in the tenant since PolicyValue is a string and can't be null. The same applies for IncludeFileExtensions, these 2 parameters by default are null in the tenant and the cmdlet should be able to restore this but can't. FYI, I was able to resolve this by running the following M365 CLI command on a PS prompt:

m365 spo cdn policy set --cdnType Public --policy ExcludeRestrictedSiteClassifications --value @()

Also note that even though that command solved my issue it also has a problem since applying it with $null instead of @() the value it stores in the tenant is TRUE which is incorrect, but that is a separate issue that should be checked with M365 CLI's team.

Expected behavior

Set-PnPTenantCdnPolicy should accept for both IncludeFileExtensions and ExcludeRestrictedSiteClassifications a null policy value, both parameters should have AllowEmptyString in https://github.com/pnp/powershell/blob/dev/src/Commands/Admin/SetTenantCdnPolicy.cs

Actual behavior

Set-PnPTenantCdnPolicy fails if policy value parameter is null since it's a string and mandatory

Steps to reproduce behavior

Set-PnPTenantCdnPolicy -CdnType "Public" `
    -PolicyType 'IncludeFileExtensions' `
    -PolicyValue $null

or

Set-PnPTenantCdnPolicy -CdnType "Public" `
    -PolicyType 'ExcludeRestrictedSiteClassifications' `
    -PolicyValue $null

What is the version of the Cmdlet module you are running?

1.12.0, the reason for the old version is due to a compatibility issue with M365DSC and therefore cannot be upgraded, but the problem is present on latest version of https://github.com/pnp/powershell/blob/dev/src/Commands/Admin/SetTenantCdnPolicy.cs

Which operating system/environment are you running PnP PowerShell on?

  • Windows
  • Linux
  • MacOS
  • Azure Cloud Shell
  • Azure Functions
  • Other : please specify
@ricmestre ricmestre added the bug Something isn't working label May 3, 2024
@ricmestre
Copy link
Author

By the way, since I described that M365DSC needs to be stuck with 1.12 for now is it possible to solve this issue with a new release on the 1.X branch?

@gautamdsheth
Copy link
Collaborator

@ricmestre - have you tried setting empty string value, "" or [string]::Empty ? We use CSOM SDK under the hood and it only accepts string values.

@jackpoz
Copy link
Contributor

jackpoz commented May 5, 2024

I did some tests with CSOM and opened a PR to allow $null and empty string as values.

@ricmestre
Copy link
Author

@jackpoz LGTM, just wanted to know as I asked before if it'd be possible to integrate this on a new release on 1.X branch or not? Not critical if it isn't, just asking.

@gautamdsheth The tenant by default may come with those properties set to null, if you change them and try to set them as null with PnP PS as of right now as explained won't work since it doesn't accept empty or null like you're asking. M365 CLI accepts it and I was able to solve my issue for now with it.

@jackpoz
Copy link
Contributor

jackpoz commented May 10, 2024

My personal opinion, not speaking for PnP: 1.x is no longer maintained and this fix will no cause a new release.

https://github.com/pnp/powershell?tab=readme-ov-file#important---new-pnp-powershell-2x is the official statement related to 1.x and 2.x .

1.12 was last released on November 2022 and it would be really good to upgrade to 2.x , even if it's still used in a lot of products.

@gautamdsheth
Copy link
Collaborator

This has been fixed with the PR by @jackpoz !

As he mentioned above, we dont support 1.x , we only make changes to the latest version.
1.x is 1.5 years old , would highly recommend you to update to 2.x which only works in PS 7.2 or later . The 2.x version also contains quite a few security fixes as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants