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

Intune vpn configuration policy android #5503

Merged
merged 12 commits into from
Dec 10, 2024

Conversation

dannyKBjj
Copy link
Contributor

Pull Request (PR) description

Adds support for microsoft.graph.androidDeviceOwnerVpnConfiguration and microsoft.graph.androidVpnConfiguration resource type.

This Pull Request (PR) fixes the following issues

None

Note: resource type and actual display in Intune GUI is reversed - i.e. microsoft.graph.androidDeviceOwnerVpnConfiguration handles Enterprise policies. Have named resources in line with resource type.

Task list

  • Added an entry to the change log under the Unreleased section of the file CHANGELOG.md.
    Entry should say what was changed and how that affects users (if applicable), and
    reference the issue being resolved (if applicable).
  • Resource parameter descriptions added/updated in the schema.mof.
  • Resource documentation added/updated in README.md.
  • Resource settings.json file contains all required permissions.
  • Examples appropriately added/updated.
  • Unit tests added/updated.
  • New/changed code adheres to DSC Community Style Guidelines.

@dannyKBjj
Copy link
Contributor Author

Unit tests say

Error: [-] Should compile example 'IntuneVPNConfigurationPolicyAndroidDeviceOwner\1-Create.ps1' correctly 696ms (695ms|1ms)
Message
Error: [-] Should compile example 'IntuneVPNConfigurationPolicyAndroidDeviceOwner\2-Update.ps1' correctly 731ms (731ms|0ms)

However, I'm able to compile the examples just fine...

@ricmestre
Copy link
Contributor

@dannyKBjj Welcome to the DSC world! The problem is that the CIM instance MSFT_targetedMobileApps is already defined somewhere else so if you reuse it with exactly the same name it must follow the same number of properties and they must also have the same names/types.

In this case it was actually you who defined this CIM instance in MSFT_IntuneVPNConfigurationPolicyIOS and instead of name you called it address, most likely not your intention. Assuming that you didn't want to do that just go and replace address by name in MSFT_IntuneVPNConfigurationPolicyIOS and the examples included in this PR should work.

image

@dannyKBjj
Copy link
Contributor Author

dannyKBjj commented Dec 4, 2024

Ah!! See #5509

I realised whilst doing my Android modules that I'd messed up defining the targetMobileApps in my iOS VPN module. I've got another pull request that'll bring them into line with each other. I don't actually think targetedMobileApps is used for iOS VPN despite being documented as belonging to the resource class (at least I can't see where it could ever be set) and therefore the issue never came up in testing. It did however come up when I tested my Android VPN configurations, so I fixed it in both. 'name' is the correct value:

see: https://learn.microsoft.com/en-us/graph/api/resources/intune-deviceconfig-applistitem?view=graph-rest-beta

I guess I should leave this until pull request 5509 is approved and then this issue will fix itself?

@ricmestre
Copy link
Contributor

Yes, just went through the other PR, if that one gets merged this will fix itself once you merge the Dev branch into it with those updates but you'll have to rely on the fact that the other PR needs to be merged first.

@ykuijs
Copy link
Member

ykuijs commented Dec 4, 2024

Other PR is merged. Please update with Dev to trigger the unit tests again

@dannyKBjj
Copy link
Contributor Author

thanks, seems to have passed the unit tests this time.

CHANGELOG.md Outdated
* Initial release
* IntuneVPNConfigurationPolicyAndroidEnterprise
* Initial release
* M365DSCDRGUtil
Copy link
Contributor

Choose a reason for hiding this comment

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

This was part of a PR from @FabienTschanz which was already merged, please remove these 2 entries from the changelog

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

  • IntuneVPNConfigurationPolicyAndroidEnterprise
    • Initial release

Copy link
Contributor

Choose a reason for hiding this comment

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

No, that's wrong. Add back your entry, what you need to remove is M365DSCDRGUtil/M365DSCUtil entries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, not sure how they ended up as part of my commit, but I've removed them! Hopefully done the right thing this time?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah looks ok now :)

@ykuijs ykuijs merged commit d775ebc into microsoft:Dev Dec 10, 2024
2 checks passed
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.

3 participants