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

test(map): standardise map.jinja verification #193

Merged

Conversation

myii
Copy link
Member

@myii myii commented Aug 26, 2020

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

UPDATE: Automated using myii/ssf-formula#281

saltstack-formulas/sudoers-formula#69

Describe the changes you're proposing

Discussion in Slack: https://freenode.logbot.info/saltstack-formulas/20200826#c4886021-c4887283.

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@pull-assistant
Copy link

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     test(_mapdata_spec): perform comparison using describe yaml instead

Powered by Pull Assistant. Last update 117443e ... 117443e. Read the comment docs.

@myii myii requested a review from baby-gnu August 26, 2020 22:41
Comment on lines 8 to 11
{%- set output_file = '/tmp/salt_mapdata_dump.yaml' %}
{%- set map = mapdata %}
{%- do salt['log.debug']('### MAP.JINJA DUMP ###\n' ~ map | yaml(False)) %}

{%- do salt['log.debug']( mapdata | yaml(False) ) %}
{%- set output_file = '/tmp/salt_mapdata_dump.yaml' %}
Copy link
Member Author

Choose a reason for hiding this comment

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

The restructuring of this section here is to find a common solution across formulas with and without the newest map.jinja. That way, it will be easier to manage this file from the ssf-formula.

Copy link
Contributor

Choose a reason for hiding this comment

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

For template-formula I limit the difference between previous and new map.jinja to the import line:

{%- from tplroot ~ "/map.jinja" import TEMPLATE as mapdata with context %}

Now, lets choose which one you prefer ;-)

Comment on lines 16 to 17
describe yaml('/tmp/salt_mapdata_dump.yaml').params do
it { should eq mapdata_dump }
Copy link
Contributor

Choose a reason for hiding this comment

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

I made a test to see how it outputs diff with the following changes:

diff --git a/test/integration/default/files/_mapdata/debian-10.yaml b/test/integration/default/files/_mapdata/debian-10.yaml
index 4bb86c2..31b6bc3 100644
--- a/test/integration/default/files/_mapdata/debian-10.yaml
+++ b/test/integration/default/files/_mapdata/debian-10.yaml
@@ -8,16 +8,10 @@ map_jinja:
   - ssh_config
 openssh:
   absent_dsa_keys: false
-  absent_ecdsa_keys: false
+  absent_ecdsa_keys: true
   absent_ed25519_keys: false
   absent_rsa_keys: false
   auth:
-    joe-non-valid-ssh-key:
-    - comment: obsolete key - removed
-      enc: ssh-rsa
-      present: false
-      source: salt://ssh_keys/joe.no-valid.pub
-      user: joe
     joe-valid-ssh-key-desktop:
     - comment: main key - desktop
       enc: ssh-rsa

gives this output:

  ×  `map.jinja` YAML dump: should contain exactly the same data as the comparison file
     ×  {"map_jinja"=>{"config_get_roots"=>["openssh", "sshd_config", "ssh_config"]}, "openssh"=>{"absent_dsa_keys"=>false, "absent_ecdsa_keys"=>false, "absent_ed25519_keys"=>false, "absent_rsa_keys"=>false, "auth"=>{"joe-non-valid-ssh-key"=>[{"comment"=>"obsolete key - removed", "enc"=>"ssh-rsa", "present"=>false, "source"=>"salt://ssh_keys/joe.no-valid.pub", "user"=>"joe"}], "joe-valid-ssh-key-desktop"=>[{"comment"=>"main key - desktop", "enc"=>"ssh-rsa", "present"=>true, "source"=>"salt://ssh_keys/joe.desktop.pub", "user"=>"joe"}], "joe-valid-ssh-key-notebook"=>[{"comment"=>"main key - notebook", "enc"=>"ssh-rsa", "present"=>true, "source"=>"salt://ssh_keys/joe.netbook.pub", "user"=>"joe"}]}, "auth_map"=>{"personal_keys"=>{"source"=>"salt://ssh_keys", "users"=>{"joe"=>{"joe.desktop"=>{}, "joe.netbook"=>{"options"=>[]}, "joe.no-valid"=>{"present"=>false}}}}}, "banner"=>"/etc/ssh/banner", "banner_src"=>"banner", "banner_string"=>"Welcome to 260cb7838fa2! ", "client"=>"openssh-client", "client_version"=>"latest", "dig_pkg"=>"dnsutils", "dsa"=>{"private_key"=>"-----BEGIN DSA PRIVATE KEY-----\nNOT_DEFINED\n-----END DSA PRIVATE KEY----- ", "public_key"=>"ssh-dss NOT_DEFINED "}, "ecdsa"=>{"private_key"=>"-----BEGIN EC PRIVATE KEY-----\nNOT_DEFINED\n-----END EC PRIVATE KEY----- ", "public_key"=>"ecdsa-sha2-nistp256 NOT_DEFINED "}, "ed25519"=>{"private_key"=>"-----BEGIN OPENSSH PRIVATE KEY-----\nNOT_DEFINED\n-----END OPENSSH PRIVATE KEY----- ", "public_key"=>"ssh-ed25519 NOT_DEFINED "}, "enforce_rsa_size"=>false, "generate_dsa_keys"=>false, "generate_ecdsa_keys"=>false, "generate_ed25519_keys"=>false, "generate_rsa_keys"=>false, "generate_rsa_size"=>4096, "host_key_algos"=>"ecdsa,ed25519,rsa", "known_hosts"=>{"aliases"=>["cname-to-minion.example.org", "alias.example.org"], "hostnames"=>false, "include_localhost"=>false, "mine_hostname_function"=>"public_ssh_hostname", "mine_keys_function"=>"public_ssh_host_keys", "omit_ip_address"=>["github.com"], "salt_ssh"=>{"public_ssh_host_keys"=>{"minion.id"=>"ssh-rsa [...]\nssh-ed25519 [...] "}, "public_ssh_host_names"=>{"minion.id"=>["minion.id", "alias.of.minion.id"]}, "user"=>"salt-master"}, "static"=>{"github.com"=>"ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGm[...]", "gitlab.com"=>"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCsj2bN[...]"}, "target"=>"*", "tgt_type"=>"glob"}, "moduli"=>"# Time Type Tests Tries Size Generator Modulus\n20120821045639 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C604293680B09D63\n20120821045830 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C6042936814C2FFB\n20120821050046 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C60429368214FC53\n20120821050054 2 6 100 2047 5 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C60429368218E83F ", "provide_dsa_keys"=>false, "provide_ecdsa_keys"=>false, "provide_ed25519_keys"=>false, "provide_rsa_keys"=>false, "root_group"=>"root", "rsa"=>{"private_key"=>"-----BEGIN RSA PRIVATE KEY-----\nNOT_DEFINED\n-----END RSA PRIVATE KEY----- ", "public_key"=>"ssh-rsa NOT_DEFINED "}, "server"=>"openssh-server", "server_version"=>"latest", "service"=>"ssh", "ssh_config"=>"/etc/ssh/ssh_config", "ssh_config_backup"=>true, "ssh_config_group"=>"root", "ssh_config_mode"=>"644", "ssh_config_src"=>"ssh_config", "ssh_config_user"=>"root", "ssh_known_hosts"=>"/etc/ssh/ssh_known_hosts", "ssh_known_hosts_src"=>"ssh_known_hosts", "ssh_moduli"=>"/etc/ssh/moduli", "sshd_binary"=>"/usr/sbin/sshd", "sshd_config"=>"/etc/ssh/sshd_config", "sshd_config_backup"=>true, "sshd_config_group"=>"root", "sshd_config_mode"=>"644", "sshd_config_src"=>"sshd_config", "sshd_config_user"=>"root", "sshd_enable"=>true}, "ssh_config"=>{"Hosts"=>{"*"=>{"GSSAPIAuthentication"=>"yes", "HashKnownHosts"=>"yes", "SendEnv"=>"LANG LC_*"}}}, "sshd_config"=>{"AcceptEnv"=>"LANG LC_*", "ChallengeResponseAuthentication"=>"no", "PrintMotd"=>"no", "Subsystem"=>"sftp /usr/lib/openssh/sftp-server", "UsePAM"=>"yes", "X11Forwarding"=>"yes"}} is expected to eq {"map_jinja"=>{"config_get_roots"=>["openssh", "sshd_config", "ssh_config"]}, "openssh"=>{"absent_dsa..."=>"no", "Subsystem"=>"sftp /usr/lib/openssh/sftp-server", "UsePAM"=>"yes", "X11Forwarding"=>"yes"}}
     
     expected: {"map_jinja"=>{"config_get_roots"=>["openssh", "sshd_config", "ssh_config"]}, "openssh"=>{"absent_dsa..."=>"no", "Subsystem"=>"sftp /usr/lib/openssh/sftp-server", "UsePAM"=>"yes", "X11Forwarding"=>"yes"}}
          got: {"map_jinja"=>{"config_get_roots"=>["openssh", "sshd_config", "ssh_config"]}, "openssh"=>{"absent_dsa..."=>"no", "Subsystem"=>"sftp /usr/lib/openssh/sftp-server", "UsePAM"=>"yes", "X11Forwarding"=>"yes"}}
     
     (compared using ==)
     
     Diff:
     @@ -1,5 +1,5 @@
      "map_jinja" => {"config_get_roots"=>["openssh", "sshd_config", "ssh_config"]},
     -"openssh" => {"absent_dsa_keys"=>false, "absent_ecdsa_keys"=>true, "absent_ed25519_keys"=>false, "absent_rsa_keys"=>false, "auth"=>{"joe-valid-ssh-key-desktop"=>[{"comment"=>"main key - desktop", "enc"=>"ssh-rsa", "present"=>true, "source"=>"salt://ssh_keys/joe.desktop.pub", "user"=>"joe"}], "joe-valid-ssh-key-notebook"=>[{"comment"=>"main key - notebook", "enc"=>"ssh-rsa", "present"=>true, "source"=>"salt://ssh_keys/joe.netbook.pub", "user"=>"joe"}]}, "auth_map"=>{"personal_keys"=>{"source"=>"salt://ssh_keys", "users"=>{"joe"=>{"joe.desktop"=>{}, "joe.netbook"=>{"options"=>[]}, "joe.no-valid"=>{"present"=>false}}}}}, "banner"=>"/etc/ssh/banner", "banner_src"=>"banner", "banner_string"=>"Welcome to 260cb7838fa2! ", "client"=>"openssh-client", "client_version"=>"latest", "dig_pkg"=>"dnsutils", "dsa"=>{"private_key"=>"-----BEGIN DSA PRIVATE KEY-----\nNOT_DEFINED\n-----END DSA PRIVATE KEY----- ", "public_key"=>"ssh-dss NOT_DEFINED "}, "ecdsa"=>{"private_key"=>"-----BEGIN EC PRIVATE KEY-----\nNOT_DEFINED\n-----END EC PRIVATE KEY----- ", "public_key"=>"ecdsa-sha2-nistp256 NOT_DEFINED "}, "ed25519"=>{"private_key"=>"-----BEGIN OPENSSH PRIVATE KEY-----\nNOT_DEFINED\n-----END OPENSSH PRIVATE KEY----- ", "public_key"=>"ssh-ed25519 NOT_DEFINED "}, "enforce_rsa_size"=>false, "generate_dsa_keys"=>false, "generate_ecdsa_keys"=>false, "generate_ed25519_keys"=>false, "generate_rsa_keys"=>false, "generate_rsa_size"=>4096, "host_key_algos"=>"ecdsa,ed25519,rsa", "known_hosts"=>{"aliases"=>["cname-to-minion.example.org", "alias.example.org"], "hostnames"=>false, "include_localhost"=>false, "mine_hostname_function"=>"public_ssh_hostname", "mine_keys_function"=>"public_ssh_host_keys", "omit_ip_address"=>["github.com"], "salt_ssh"=>{"public_ssh_host_keys"=>{"minion.id"=>"ssh-rsa [...]\nssh-ed25519 [...] "}, "public_ssh_host_names"=>{"minion.id"=>["minion.id", "alias.of.minion.id"]}, "user"=>"salt-master"}, "static"=>{"github.com"=>"ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGm[...]", "gitlab.com"=>"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCsj2bN[...]"}, "target"=>"*", "tgt_type"=>"glob"}, "moduli"=>"# Time Type Tests Tries Size Generator Modulus\n20120821045639 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C604293680B09D63\n20120821045830 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C6042936814C2FFB\n20120821050046 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C60429368214FC53\n20120821050054 2 6 100 2047 5 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C60429368218E83F ", "provide_dsa_keys"=>false, "provide_ecdsa_keys"=>false, "provide_ed25519_keys"=>false, "provide_rsa_keys"=>false, "root_group"=>"root", "rsa"=>{"private_key"=>"-----BEGIN RSA PRIVATE KEY-----\nNOT_DEFINED\n-----END RSA PRIVATE KEY----- ", "public_key"=>"ssh-rsa NOT_DEFINED "}, "server"=>"openssh-server", "server_version"=>"latest", "service"=>"ssh", "ssh_config"=>"/etc/ssh/ssh_config", "ssh_config_backup"=>true, "ssh_config_group"=>"root", "ssh_config_mode"=>"644", "ssh_config_src"=>"ssh_config", "ssh_config_user"=>"root", "ssh_known_hosts"=>"/etc/ssh/ssh_known_hosts", "ssh_known_hosts_src"=>"ssh_known_hosts", "ssh_moduli"=>"/etc/ssh/moduli", "sshd_binary"=>"/usr/sbin/sshd", "sshd_config"=>"/etc/ssh/sshd_config", "sshd_config_backup"=>true, "sshd_config_group"=>"root", "sshd_config_mode"=>"644", "sshd_config_src"=>"sshd_config", "sshd_config_user"=>"root", "sshd_enable"=>true},
     +"openssh" => {"absent_dsa_keys"=>false, "absent_ecdsa_keys"=>false, "absent_ed25519_keys"=>false, "absent_rsa_keys"=>false, "auth"=>{"joe-non-valid-ssh-key"=>[{"comment"=>"obsolete key - removed", "enc"=>"ssh-rsa", "present"=>false, "source"=>"salt://ssh_keys/joe.no-valid.pub", "user"=>"joe"}], "joe-valid-ssh-key-desktop"=>[{"comment"=>"main key - desktop", "enc"=>"ssh-rsa", "present"=>true, "source"=>"salt://ssh_keys/joe.desktop.pub", "user"=>"joe"}], "joe-valid-ssh-key-notebook"=>[{"comment"=>"main key - notebook", "enc"=>"ssh-rsa", "present"=>true, "source"=>"salt://ssh_keys/joe.netbook.pub", "user"=>"joe"}]}, "auth_map"=>{"personal_keys"=>{"source"=>"salt://ssh_keys", "users"=>{"joe"=>{"joe.desktop"=>{}, "joe.netbook"=>{"options"=>[]}, "joe.no-valid"=>{"present"=>false}}}}}, "banner"=>"/etc/ssh/banner", "banner_src"=>"banner", "banner_string"=>"Welcome to 260cb7838fa2! ", "client"=>"openssh-client", "client_version"=>"latest", "dig_pkg"=>"dnsutils", "dsa"=>{"private_key"=>"-----BEGIN DSA PRIVATE KEY-----\nNOT_DEFINED\n-----END DSA PRIVATE KEY----- ", "public_key"=>"ssh-dss NOT_DEFINED "}, "ecdsa"=>{"private_key"=>"-----BEGIN EC PRIVATE KEY-----\nNOT_DEFINED\n-----END EC PRIVATE KEY----- ", "public_key"=>"ecdsa-sha2-nistp256 NOT_DEFINED "}, "ed25519"=>{"private_key"=>"-----BEGIN OPENSSH PRIVATE KEY-----\nNOT_DEFINED\n-----END OPENSSH PRIVATE KEY----- ", "public_key"=>"ssh-ed25519 NOT_DEFINED "}, "enforce_rsa_size"=>false, "generate_dsa_keys"=>false, "generate_ecdsa_keys"=>false, "generate_ed25519_keys"=>false, "generate_rsa_keys"=>false, "generate_rsa_size"=>4096, "host_key_algos"=>"ecdsa,ed25519,rsa", "known_hosts"=>{"aliases"=>["cname-to-minion.example.org", "alias.example.org"], "hostnames"=>false, "include_localhost"=>false, "mine_hostname_function"=>"public_ssh_hostname", "mine_keys_function"=>"public_ssh_host_keys", "omit_ip_address"=>["github.com"], "salt_ssh"=>{"public_ssh_host_keys"=>{"minion.id"=>"ssh-rsa [...]\nssh-ed25519 [...] "}, "public_ssh_host_names"=>{"minion.id"=>["minion.id", "alias.of.minion.id"]}, "user"=>"salt-master"}, "static"=>{"github.com"=>"ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGm[...]", "gitlab.com"=>"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCsj2bN[...]"}, "target"=>"*", "tgt_type"=>"glob"}, "moduli"=>"# Time Type Tests Tries Size Generator Modulus\n20120821045639 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C604293680B09D63\n20120821045830 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C6042936814C2FFB\n20120821050046 2 6 100 2047 2 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C60429368214FC53\n20120821050054 2 6 100 2047 5 DD2047CBDBB6F8E919BC63DE885B34D0FD6E3DB2887D8B46FE249886ACED6B46DFCD5553168185FD376122171CD8927E60120FA8D01F01D03E58281FEA9A1ABE97631C828E41815F34FDCDF787419FE13A3137649AA93D2584230DF5F24B5C00C88B7D7DE4367693428C730376F218A53E853B0851BAB7C53C15DA7839CBE1285DB63F6FA45C1BB59FE1C5BB918F0F8459D7EF60ACFF5C0FA0F3FCAD1C5F4CE4416D4F4B36B05CDCEBE4FB879E95847EFBC6449CD190248843BC7EDB145FBFC4EDBB1A3C959298F08F3BA2CFBE231BBE204BE6F906209D28BD4820AB3E7BE96C26AE8A809ADD8D1A5A0B008E9570FA4C4697E116B8119892C60429368218E83F ", "provide_dsa_keys"=>false, "provide_ecdsa_keys"=>false, "provide_ed25519_keys"=>false, "provide_rsa_keys"=>false, "root_group"=>"root", "rsa"=>{"private_key"=>"-----BEGIN RSA PRIVATE KEY-----\nNOT_DEFINED\n-----END RSA PRIVATE KEY----- ", "public_key"=>"ssh-rsa NOT_DEFINED "}, "server"=>"openssh-server", "server_version"=>"latest", "service"=>"ssh", "ssh_config"=>"/etc/ssh/ssh_config", "ssh_config_backup"=>true, "ssh_config_group"=>"root", "ssh_config_mode"=>"644", "ssh_config_src"=>"ssh_config", "ssh_config_user"=>"root", "ssh_known_hosts"=>"/etc/ssh/ssh_known_hosts", "ssh_known_hosts_src"=>"ssh_known_hosts", "ssh_moduli"=>"/etc/ssh/moduli", "sshd_binary"=>"/usr/sbin/sshd", "sshd_config"=>"/etc/ssh/sshd_config", "sshd_config_backup"=>true, "sshd_config_group"=>"root", "sshd_config_mode"=>"644", "sshd_config_src"=>"sshd_config", "sshd_config_user"=>"root", "sshd_enable"=>true},
      "ssh_config" => {"Hosts"=>{"*"=>{"GSSAPIAuthentication"=>"yes", "HashKnownHosts"=>"yes", "SendEnv"=>"LANG LC_*"}}},
      "sshd_config" => {"AcceptEnv"=>"LANG LC_*", "ChallengeResponseAuthentication"=>"no", "PrintMotd"=>"no", "Subsystem"=>"sftp /usr/lib/openssh/sftp-server", "UsePAM"=>"yes", "X11Forwarding"=>"yes"},

Not really easy for a human to spot the differences.

Comment on lines 8 to 11
{%- set output_file = '/tmp/salt_mapdata_dump.yaml' %}
{%- set map = mapdata %}
{%- do salt['log.debug']('### MAP.JINJA DUMP ###\n' ~ map | yaml(False)) %}

{%- do salt['log.debug']( mapdata | yaml(False) ) %}
{%- set output_file = '/tmp/salt_mapdata_dump.yaml' %}
Copy link
Contributor

Choose a reason for hiding this comment

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

For template-formula I limit the difference between previous and new map.jinja to the import line:

{%- from tplroot ~ "/map.jinja" import TEMPLATE as mapdata with context %}

Now, lets choose which one you prefer ;-)

@myii
Copy link
Member Author

myii commented Aug 28, 2020

I made a test to see how it outputs diff with the following changes:

...

Not really easy for a human to spot the differences.


UPDATE: The diff situations has changed with the latest gems, see saltstack-formulas/openvpn-formula#132 (comment).


@baby-gnu So we've got a decision to make about whether we compare textually or using the YAML itself. This affects more than just this PR -- rather the way we do this across all of the formulas. The YAML method came about when I encountered problems doing this verification on Windows in the salt-formula. I had to resort to using gsub to match up the line endings (\r\n instead of \n). @dafyddj suggested matching by data structure and @daks provided the reference to matching the YAML. So trying to lay this out to help finalise what to use:

Factor Text (current) YAML (proposed)
InSpec diff output [+] Line-by-line, much easier to see the differences [-] Partial data structure, difficult to identify the differences
Windows [-] Hacky, have to resort to using gsub for \n => \r\n [+] Works in the same way as on Linux, no workarounds required
Comparison file ordering [?] Must be exact (alphabetical throughout) [?] As long as the data is correct, the ordering doesn’t matter

The last point ([?]) may actually work out as an advantage for the text-based method. By keeping all of the comparison files ordered alphabetically, working with/across the files becomes much easier. There have been times when you need to compare across the comparison files themselves, to see what changes (such as across releases such as ubuntu-16, ubuntu-18 & ubuntu-20).

So it seems that from a human perspective, using the text-based comparison actually works out better. The hack only has to be added in one place and that will probably end up controlled by the ssf-formula anyway.

@dafyddj
Copy link
Contributor

dafyddj commented Oct 15, 2020

Not sure where to put this, but I've seen applying this to Windows discussed somewhere and this is my attempt:

diff --git a/TEMPLATE/_mapdata/init.sls b/TEMPLATE/_mapdata/init.sls
index cfa4837..b998360 100644
--- a/TEMPLATE/_mapdata/init.sls
+++ b/TEMPLATE/_mapdata/init.sls
@@ -7,7 +7,8 @@
 
 {%- do salt['log.debug']('### MAP.JINJA DUMP ###\n' ~ mapdata | yaml(False)) %}
 
-{%- set output_file = '/tmp/salt_mapdata_dump.yaml' %}
+{%- set tmp = {'Windows': 'TMP'}[grains['kernel']] | default('TMPDIR') %}
+{%- set output_file = salt['environ.get'](tmp) ~ '/salt_mapdata_dump.yaml' %}
 
 {{ tplroot }}-mapdata-dump:
   file.managed:
diff --git a/test/integration/default/controls/_mapdata_spec.rb b/test/integration/default/controls/_mapdata_spec.rb
index 2e287f2..5ed4192 100644
--- a/test/integration/default/controls/_mapdata_spec.rb
+++ b/test/integration/default/controls/_mapdata_spec.rb
@@ -9,8 +9,25 @@ mapdata_dump = inspec.profile.file(mapdata_file)
 control '`map.jinja` YAML dump' do
   title 'should contain the lines'
 
-  describe file('/tmp/salt_mapdata_dump.yaml') do
+  tmp =
+    case platform[:family]
+    when 'windows'
+      'TMP'
+    else
+      'TMPDIR'
+    end
+
+  mapdata_file = file("#{os_env(tmp).content}/salt_mapdata_dump.yaml")
+
+  describe mapdata_file do
     it { should exist }
-    its('content') { should eq mapdata_dump }
+  end
+
+  mapdata_content = mapdata_file.content.encode(universal_newline: true)
+
+  describe 'File content' do
+    it 'should match profile map data exactly' do
+      expect(mapdata_content).to eq(mapdata_dump)
+    end
   end
 end

@baby-gnu
Copy link
Contributor

Not sure where to put this, but I've seen applying this to Windows discussed somewhere and this is my attempt:

Thanks @dafyddj, there is no TMP or TMPDIR in my GNU/Linux minion environments so it should a little more complicated to support both.

I would more something like:

diff --git a/openssh/_mapdata/init.sls b/openssh/_mapdata/init.sls
index 5e4fcf1..46cf87f 100644
--- a/openssh/_mapdata/init.sls
+++ b/openssh/_mapdata/init.sls
@@ -5,7 +5,13 @@
 {%- set tplroot = tpldir.split('/')[0] %}
 {%- from tplroot ~ "/map.jinja" import mapdata with context %}
 
-{%- set output_file = '/tmp/salt_mapdata_dump.yaml' %}
+{%- if salt["grains.get"]("kernel") == 'Windows' %}
+{%-   set tmp_dir = salt["environ.get"]("TMP") %}
+{%- else %}
+{%-   set tmp_dir = '/tmp' %}
+{%- endif %}
+
+{%- set output_file = tmp_dir ~ '/salt_mapdata_dump.yaml' %}
 
 {%- do salt['log.debug']( mapdata | yaml(False) ) %}

But thanks a lot for the mapdata_file.content.encode(universal_newline: true) I was not aware of it ;-)

@myii
Copy link
Member Author

myii commented Oct 16, 2020

@dafyddj The strange thing about this path issue is that it actually worked as-is during my Windows testing in the salt-formula a couple of months ago:

https://github.com/myii/salt-formula/runs/1032273456?check_suite_focus=true#step:8:1532

      [PASS]  File /tmp/salt_mapdata_dump.yaml content is expected to eq "# yamllint disable rule:indentation rule:line-length\r\n# Windows-2019Server\r\n---\r\nformulas_sett...yndic\r\n  ssh_roster: {}\r\n  syndic_service: salt-syndic\r\n  use_pip: false\r\n  version: ''\r\n"
{%- set output_file = '/tmp/salt_mapdata_dump.yaml' %}
+{%- if salt["grains.get"]("kernel") == 'Windows' %}
+{%-   set tmp_dir = salt["environ.get"]("TMP") %}
+{%- else %}
+{%-   set tmp_dir = '/tmp' %}
+{%- endif %}

@baby-gnu Out of interest, is there any specific benefit to using grains.kernel over grains.os_family (which is also Windows)?

@baby-gnu
Copy link
Contributor

@dafyddj The strange thing about this path issue is that it actually worked as-is during my Windows testing in the salt-formula a couple of months ago:

I'll recheck because on my windows systems there is no c:\tmp. Maybe it came with some tooling installed for the CI? I only try to run the formula on a pristine windows 10 machine + salt-minion.

@baby-gnu Out of interest, is there any specific benefit to using grains.kernel over grains.os_family (which is also Windows)?

In that specific case, absolutely no benefit, I used grains["kernel"] somewhere else to distinguish between windows and all linux systems. It was in my mind and my fingers type that without much conscious thinking ;-)

NB: sorry, I edited your comment instead of quote reply, I think I need a nap ;-)

@myii
Copy link
Member Author

myii commented Oct 21, 2020

I've updated my comment above (#193 (comment)) because the diff is no longer showing when doing a text-based comparison using eq.

@myii myii force-pushed the test/compare-mapdata-using-yaml branch from 117443e to 9303355 Compare December 21, 2020 23:45
@myii myii requested a review from a team as a code owner December 21, 2020 23:45
@myii myii changed the title test(_mapdata_spec): perform comparison using describe yaml instead test(map): standardise map.jinja verification Dec 21, 2020
@myii
Copy link
Member Author

myii commented Dec 21, 2020

Repurposed this PR with all of the standardisations worked out across the various formulas.

@myii myii force-pushed the test/compare-mapdata-using-yaml branch from 9303355 to 2bab68f Compare December 22, 2020 00:45
@myii myii merged commit a7d9ae2 into saltstack-formulas:master Dec 22, 2020
@myii
Copy link
Member Author

myii commented Dec 22, 2020

Self-merging since tests only.

@myii myii deleted the test/compare-mapdata-using-yaml branch December 22, 2020 12:35
@saltstack-formulas-travis

🎉 This PR is included in version 2.0.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants