grahampugh / jamf-upload

Scripts for uploading packages to Jamf Cloud
Apache License 2.0
149 stars 37 forks source link

Jamf Uploader Feature request: Optionally disable XML escaping for template variable substitution #104

Closed davidbpirie closed 1 year ago

davidbpirie commented 1 year ago

I would like the ability to disable XML escaping when an XML template has variable substitution performed in the Jamf Uploader processors, namely:

JamfAccountUploader JamfMacAppUploader JamfPatchUploader JamfPolicyUploader JamfSoftwareRestrictionUploader My use case is to reduce the number of templates I need to maintain in our environment and move more flexibility into our AutoPkg recipe overrides, for example including 2 computer groups and corresponding XML tags directly in a single variable that is substituted into the template.

In my environment I try to re-use policy templates as much as possible, rather than creating one for each recipe override. We ended up needing several templates where the only differences were to do with scoping, in particular with a varying number of substitutions. For example, one template for where there were no exclusion computer groups, another where there was one, and another where there was two etc.

My solution to simplify was to effectively include multiple computer group names, along with their XML tags, in a single input variable. However jamf-upload was automatically escaping the xml inside the variable, preventing this solution. I put this code change together as a proof of concept and it served well.

As a side note, I then took it a step further and created a StringJoiner shared processor (com.github.davidbpirie.SharedProcessors/StringJoiner) that can be given list input variables and it joins the list values together with provided glue into a new single variable; if the list is empty, the result is an empty string (ie no leftover glue).

Obviously enabling this option in a recipe requires careful consideration by the author about what input variables are being substituted, and whether they contain characters that need to be escaped for XML.

I will submit a PR with my suggested implementation.

grahampugh commented 1 year ago

Hi David

This is an interesting idea but I fear that recipes using this are going to look messy and hard to describe.

Furthermore, what happens if your computer group name requires XML escaping, e.g. "Development & Computing"?

It would also introduce an inconsistency because introducing this exception into JamfComputerProfileUploader would break it, where XML escaping is always required.

davidbpirie commented 1 year ago

Hi Graham,

I disagree about the messy recipes, but it all depends on how you look at it, so I am happy to agree to disagree.

For the inconsistency, yes making use of this would require that you don't also use processors that require escaping alongside those where you disable it, and where you disable escaping you would need to handle the escaping yourself.

Admittedly I don't currently use JamfComputerProfileUploader so hadn't considered the implications for that context - profiles are the only thing remaining in our Jamf Pro instances that are not put there by AutoPkg, and it is on my future roadmap to implement.

So if I can't convince you, I have some other alternative ideas that may give me what I am after - I would just need to perform my own limited substitution processing on the template before it is passed to the Jamf Uploader processors.

grahampugh commented 1 year ago

Are you using yaml recipes? I'd be interested to see how this idea could work inside a plist recipe without having to double escape everything which would be horrendous. But perhaps I'm still missing something about your idea.

But to be honest, how the recipe looks is less important than the fact that something as simple as a category name or smart group name with an ampersand in it would break that whole workflow. This feels like a huge caveat and it would be tough to document and support in the future.

davidbpirie commented 1 year ago

OK, I did a bunch of work on this idea yesterday, and I am convinced that my suggested changes to jamf-upload are a bad idea. Thanks for pushing me to find a better solution.

As for your other questions, yes I use yaml recipes, but I had thought this through with plist recipes also and, although uglier, still works. But for completeness, the change to jamf-upload were really only half the story - although it would make it technically possible to put some fairly ugly input variables in, in practice I intended to use another processor to "glue" the values of a array/list into a single string variable, which would then be passed through.

But as I said, your prompting has pushed me to a better solution - instead all of this is handled outside jamf-upload using a new processor that does the substitution on only select values, and writes a new template file. So before I call eg JamfPolicyUploader, I would first call my FileTemplateFromLists processor, feeding it the template file. It then does the list-glueing substitutions for specified keys, without touching the others, and writes the result to a provided location (eg the recipe cache dir). When I then call JamfPolicyUploader, I specify the cache dir file as the template.

The beauty of this method is in FileTemplateFromLists works:

  1. It only keys given in the input, not just everything in self.env;
  2. Each value can have an optional prefix and/or suffix applied;
  3. The whole substitution can have an additional prefix and/or suffix applied;
  4. If the substitution key is given an empty replacement value, it is substituted in the template file with an empty string (no prefix/suffix).

Obviously my use case is not for everyone, but it means I can use a single template for most of my recipes, and centralise the variations for each in my overrides. As I (like you) run several related but separate Jamf instances, this helps simplify source control.

Anyway, I will close off this request. Thanks again for looking into it for me.

grahampugh commented 1 year ago

Hi, I'm glad you found a good solution, and thanks for understanding my concerns with your original method.

I like the idea of a separate processor for those that want to abstract further from the use of a normal template. You may remember that JSSImporter tried to abstract scope, scripts and packages out of the template and it was very hard to document and handle exceptions. With your method there's no requirement to document these more abstracted workflows.

MLBZ521 commented 10 months ago

Ah, I wasn't the only one with a somewhat similar workflow and needing to not escape XML content. 😁 #76

davidbpirie commented 10 months ago

@MLBZ521 if it is of interest to you, my Custom Processor is https://github.com/autopkg/davidbpirie-recipes/blob/main/SharedProcessors/FileTemplateFromLists.py which solved my problem from #104. Feel free to reach out on the MacAdmin Slack if you want to discuss it.

MLBZ521 commented 10 months ago

I'll keep it in mind for sure, but I think we were attempting to solve different problems and simply needed the same solution.

I am building the scope of the Policy via a Custom Processor and then inserting that into the Policy Template via Variable Substitution. Obviously the scope is an XML payload, so when that got escaped, it didn't work.