datum icon indicating copy to clipboard operation
datum copied to clipboard

Merge not working

Open schnyders opened this issue 4 years ago • 5 comments

Hi, I've a problem with the merging when using non unique roles (multiples rules assigned to one server). I've created some demo code here "ResolutionPrecedence": [ "AllNodes\\$($Node.Name)", "Role\\<%= $CurrentNode.PSObject.Properties.where{$_.Name -in $Node.Packages}.Value %>" ],

When using the "dynamic" Datum.json section "firewallrules" then contains two times "items",

{
  "FirewallRules": [
    {
      "Items": [
        {
          "Action": 2,
          "Description": "Test123",
          "Direction": 1,
          "DisplayName": "Test123",
          "RemotePort": "Any",
          "Name": "Test123",
          "LocalAddress": "Any",
          "LocalPort": "123",
          "Service": "Any",
          "Protocol": "TCP",
          "Profile": 0,
          "Program": "Any",
          "RemoteAddress": "Any",
          "Enabled": 1
        }
      ]
    },
    {
      "Items": [
        {
          "Action": 2,
          "Description": "Test456",
          "Direction": 1,
          "DisplayName": "Test456",
          "RemotePort": "Any",
          "Name": "Test456",
          "LocalAddress": "Any",
          "LocalPort": "456",
          "Service": "Any",
          "Protocol": "TCP",
          "Profile": 0,
          "Program": "Any",
          "RemoteAddress": "Any",
          "Enabled": 1
        }
      ]
    }
  ],
  "Configurations": [
    "FirewallRules",
    "FirewallRules"
  ],
  "NodeName": "SRV01",
  "Packages": [
    "Test123",
    "Test456"
  ],
  "Name": "SRV01"
}

when using Datum1.json (that has just hardcoded the two dynamic linked JSON's it just contains one "items" under "firewallrules".

{
  "Configurations": [
    "FirewallRules"
  ],
  "Packages": [
    "Test123",
    "Test456"
  ],
  "FirewallRules": {
    "Items": [
      {
        "Service": "Any",
        "Name": "Test123",
        "RemoteAddress": "Any",
        "LocalPort": "123",
        "RemotePort": "Any",
        "LocalAddress": "Any",
        "Enabled": 1,
        "Description": "Test123",
        "Action": 2,
        "Profile": 0,
        "Protocol": "TCP",
        "Program": "Any",
        "DisplayName": "Test123",
        "Direction": 1
      },
      {
        "Action": 2,
        "Description": "Test456",
        "Direction": 1,
        "DisplayName": "Test456",
        "RemotePort": "Any",
        "Name": "Test456",
        "LocalAddress": "Any",
        "LocalPort": "456",
        "Service": "Any",
        "Protocol": "TCP",
        "Profile": 0,
        "Program": "Any",
        "RemoteAddress": "Any",
        "Enabled": 1
      }
    ]
  },
  "Name": "SRV01",
  "NodeName": "SRV01"
}

schnyders avatar Jun 30 '21 16:06 schnyders

So for me it looks like datum has a problem when ResolutionPrecedence returns an array.

...
"ResolutionPrecedence": [
    "AllNodes\\$($Node.Name)",
    "Role\\<%= $CurrentNode.PSObject.Properties.where{$_.Name -in $Node.Packages}.Value %>"
  ],
...

"Role\\<%= $CurrentNode.PSObject.Properties.where{$_.Name -in $Node.Packages}.Value %>" then returns an array, and instead of handling every one individually it try's to handle the full array in once, which then ends that the result will also be an array. And then datum is unable to merge it correctly. It gets even worse if you have firewallrules items on an other level (as example on the server level itself)

any help would be very welcome.

Ups, just realized that my hardcoded example is more or less useless, but anyway the dynamic one still has the issue.

schnyders avatar Jul 07 '21 09:07 schnyders

Hiya, just saw this issue amongst the pile of notifications coming from GitHub. :)

The very short and opinionated answer is that you should not have collections of "Roles". It goes against the better approach of having a single definition that describe the mold for several machines, and only having specific overrides at location/environment/node level (each node has 1 and only 1 role, that can have some values overridden in more specific layers).

Now if you still want to go down your route, I don't think this will work in resolution precedence, and I wouldn't call it a bug either (a missing feature, at best, but I agree that the code should more gracefully handle this and warn). You may have to write a DatumHandler that does the calculation and does what you want by doing several lookup for each of your 'role'.

I hope you found the DscWorkshop that has a good example of how you could use Datum.

If I don't at least ack an issue after a week, best is probably to come and ping me on slack/discord in DSC channel.

gaelcolas avatar Jul 12 '21 20:07 gaelcolas

Ok, I'll check that. If that's a "missing feature" then maybe you should remove the comment from your DscInfraSample repo

ResolutionPrecedence:
  - 'AllNodes\$($Node.Environment)\$($Node.Name)'
  - 'Environments\$($Node.Environment)'
  - 'SiteData\$($Node.Location)'
  - 'SiteData\$($Node.Location)\Roles' #variable expansion
  - 'Roles\$($Node.Role)' #if Node has unique role, otherwise use <%= $CurrentNode.PSObject.Properties.where{$_.Name -in $Node.Role}.Value %>
  - 'Roles\All'
#  - 'AllNodes\<%= $CurrentNode.PSObject.Properties.where{$_.Name -eq $Node.Name}.Value%>\Roles' #script block execution
#  - 'AllNodes\All\Roles'

That comment sounds like datum would support multiple "Roles" per server.

schnyders avatar Jul 13 '21 07:07 schnyders

@gaelcolas, should we flag the DscInfraSample repo as archived and redirect to the DscWorkshop instead?

raandree avatar Sep 24 '21 11:09 raandree

It is the cleaner way to have just one role per node, but the reality looks a bit different. Especially in test and dev environments one may want to assign more roles to one node to save resources.

We have a number of customers that asked for this, and things get way more simple with the support of multiple roles. This is how it works with the Datum version in my fork:

Datum.yml

ResolutionPrecedence:
  - AllNodes\$($Node.Environment)\$($Node.ActualName)
  - Environments\$($Node.Environment)
  - '[x={ $Node.Role | ForEach-Object { "Roles\$_" } } =]'
  - Baselines\$($Node.Baseline)

One of the nodes:

ActualName: '[x={ $Node.Name }=]'
NodeName: '[x="$($Node.ActualName)"=]'
Environment: Dev
Role:
  - FileServer
  - WebServer
  - TestServer

Note: This requires the datum handler Datum.InvokeCommand.

I hope to be able to file a PR to move the new stuff into the main repo during the next weeks.

raandree avatar Sep 24 '21 18:09 raandree