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

Fix for issue - #5587 #5588

Open
wants to merge 2 commits into
base: Dev
Choose a base branch
from

Conversation

dannyKBjj
Copy link
Contributor

Pull Request (PR) description

MSFT_IntuneMobileAppConfigurationPolicyIOS and MSFT_IntuneAndroidManagedStoreAppConfiguration
were supposed to be pulled in favour of existing DSCResource - IntuneAppConfigurationDevicePolicy as my modules duplicated functionality. Whilst testing on our tenant we discovered the following issue: IntuneAppConfigurationDevicePolicy cannot handle 'for Intune' applications #5587

It seems that for some reason only MSFT_IntuneMobileAppConfigurationPolicyIOS was removed. This pull request adds that module back in and deletes "IntuneAppConfigurationDevicePolicy" as I believe that module is now defunct.

See
#5401
#5587

This Pull Request (PR) fixes the following issues

- Fixes #5587

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.

MSFT_IntuneMobileAppConfigurationPolicyIOS and
MSFT_IntuneAndroidManagedStoreAppConfiguration
were supposed to be pulled in favour of existing DSCResource - IntuneAppConfigurationDevicePolicy as my modules duplicated functionality. Whilst testing on our tenant we discovered the following issue:
IntuneAppConfigurationDevicePolicy cannot handle 'for Intune' applications microsoft#5587

It seems that for some reason only MSFT_IntuneMobileAppConfigurationPolicyIOS was removed. This pull request adds that module back in and deletes "IntuneAppConfigurationDevicePolicy" as I believe that module is now defunct.
@FabienTschanz
Copy link
Contributor

@dannyKBjj Removing a resource is a breaking change. The next braking change is (I believe) in April 2025. It would be better to add the functionality to IntuneAppConfigurationDevicePoliy instead of replacing it with new resources.

@dannyKBjj
Copy link
Contributor Author

Hi, but the problem is that it is using convoluted logic to handle both. It is inconsistent with almost every other module (which all handle each workload separately) and has caused an issue, who is to say there aren't other issues? Is it possible to just submit my module alongside it and remove the bugged module at a later date? I don't really want to re-write the existing module as I believe the approach I've taken is better anyway (besides which, I have a working solution already).

@FabienTschanz
Copy link
Contributor

@dannyKBjj Normally, if there is an issue with a module, we fix the issue in that. The name of the module comes from the separation between the two categories "Managed App" and "Managed Devices". These are also the two things we can select from in the Intune portal, which aligns more with the user experience someone would expect. We already had some cases where we didn't select the "correct" name by workload or by the resource type of what Graph would tell us but rather stay in the Intune portal realms with their naming.

It's normal for new features and functionality to appear. This happens all the time, so we have to adapt the existing resources for their new purpose. I'd wait until @ykuijs and @NikCharlebois are back and get their opinion as well. I'm just a public contributor adding my input. If you think your solution is better, we'll wait for their feedback. That's totally fine by me.

@dannyKBjj
Copy link
Contributor Author

dannyKBjj commented Dec 24, 2024

Hi, fair enough...

I don't think this is a new feature thing, this is just a fault in the way approach taken.

The issue is that the policy pulled by Get-MgBetaDeviceAppManagementMobileAppConfiguration uses the 'ID' of the targeted mobile app in the 'TargetedMobileApps' property, which is a unique value.

For reasons I don't quite understand existing IntuneAppConfigurationDevicePolicy doesn't use this ID directly, but instead uses it to retrieve the packageId/bundleId.

The problem is that for these apps iOS and Android will have share the same value, see below:

image

It then uses value this to retrieve the 'Id' in Set-Target (see below)
image

However, because the app/bundleId for iOS and Android is the same, it will simply retrieve both IDs and write them into the policy (which breaks it).

I'll see what is decided in the new year, but from my perspective, the general approach seems to be to write a module for each resource, as DSC doesn't store the platform information. In this case the module seems to be determining it by the presence of certain properties after the fact. Just seems unnecessarily complicated and out of step with most of the other modules.

I can fix the existing one, but seems fixing it is very much the hard route, when I've got 2 modules that already work? Particularly when one of those modules was never removed and the other was released, and then removed in favour of this one (which doesn't work).

Besides which; I feel my approach is more simple, easier to maintain and more consistent with the other DSC resources?

Anyway, have a great Xmas! I'll pick this up when I'm back in the New Year.

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.

2 participants