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.

Sign up for GitHub

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

Jump to bottom

Adding support for Windows #67

Merged
merged 10 commits into from
Feb 20, 2020
Merged

Adding support for Windows #67

merged 10 commits into from
Feb 20, 2020

Conversation

nmaludy
Copy link
Contributor

@nmaludy nmaludy commented Feb 13, 2020 •
edited
Loading

This is another PR to add support for Windows. Hopefully this is more concise than the others.

TODO

  • Need to test the null values for facter::facts_d_mode in the Windows data actually performs the Hiera lookup correctly.
  • Need to test the user/group of NT AUTHORITY\SYSTEM sets the proper user/group.
  • Fix unit tests (seems a bunch of them are broken)
  • Add spec tests for Windows OS

ghoneycutt
ghoneycutt reviewed Feb 13, 2020
View reviewed changes
data/os/windows.yaml Outdated Show resolved Hide resolved
ghoneycutt
ghoneycutt reviewed Feb 13, 2020
View reviewed changes
manifests/init.pp Outdated Show resolved Hide resolved
ghoneycutt
ghoneycutt reviewed Feb 13, 2020
View reviewed changes
manifests/init.pp Outdated Show resolved Hide resolved
ghoneycutt
ghoneycutt reviewed Feb 13, 2020
View reviewed changes
metadata.json Outdated Show resolved Hide resolved
@ghoneycutt
Copy link
Owner

@nmaludy this is awesome work, thank you very much!!

@nmaludy
Copy link
Contributor Author

nmaludy commented Feb 14, 2020

OK, i got Travis to the point where it's passing linting and running the spec tests. Most of the specs are failing currently. I'll fix those next.

ghoneycutt
ghoneycutt reviewed Feb 14, 2020
View reviewed changes
spec/fixtures/hiera/hiera.yaml
- parameter_tests/%{parameter_tests}
version: 5
defaults:
datadir: data # This path is relative to hiera.yaml's directory.
Copy link
Owner

Choose a reason for hiding this comment

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

Wouldn't this keep it from using the data in fixtures which is the point of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, sorry the spec tests are configured to use this spec/fixtures/hiera/hiera.yaml and the datadir is relative to that file's directory, so it uses: spec/fixtures/hiera/data. Does that make sense? Sorry for the confusing wording.

@nmaludy
Copy link
Contributor Author

nmaludy commented Feb 14, 2020

@ghoneycutt got tests written, the only one that is failing is https://github.com/ghoneycutt/puppet-module-facter/pull/67/files#diff-279972940b2611b2c7a177c5e1344902R133-R136

I think we do need the Optional[] around the $file and $facts_dir so we can fall back to using $facter::facts_file and $facter::facts_d_dir. Is that OK?

@nmaludy nmaludy changed the title [WIP] Adding support for Windows Adding support for Windows Feb 14, 2020
@ghoneycutt ghoneycutt merged commit c9a37da into ghoneycutt:master Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ghoneycutt ghoneycutt Awaiting requested review from ghoneycutt

Assignees
No one assigned
Labels
backwards-incompatible enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nmaludy @ghoneycutt