-
Notifications
You must be signed in to change notification settings - Fork 61
Modernize kitchen-inspec #315
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
Conversation
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
| spec.add_dependency "inspec-core", ">= 2.2.64", "< 8.0" # 2.2.64 is required for plugin v2 support & InSpec 6 included | ||
| spec.add_dependency "train" | ||
| spec.add_dependency "hashie", ">= 3.4", "< 6.0" | ||
| if ENV["CHEF_TEST_KITCHEN_ENTERPRISE"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will only work at build time and not at runtime is this expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, when building Chef Test-Kitchen Enterprise this gem will be pulled in and built and this will ensure CTKE satisfies it'd dependency, while maintaining backward compatibility with community test-kitchen.
| spec.add_dependency "inspec", ">= 2.2.64", "< 7.0" # 2.2.64 is required for plugin v2 support & InSpec 6 included | ||
| spec.add_dependency "test-kitchen", ">= 2.7", "< 4" # 2.7 introduced no_parallel_for for verifiers | ||
| spec.add_dependency "hashie", ">= 3.4", "<= 5.0" | ||
| spec.add_dependency "inspec-core", ">= 2.2.64", "< 8.0" # 2.2.64 is required for plugin v2 support & InSpec 6 included |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be a breaking change if you have previously relied on these trains https://github.com/inspec/inspec/blob/main/inspec.gemspec#L45 ( Line 45 - 48 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's why the train dependency was added below it to enable the various connection mechanisms for containers, ssh, winrm etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if you see the link I tagged above, it has resources other than core, like aws, habitat, winrm & kubernetes. If any user had been using them post this release they have to explicitly add those gems .. may be we should show that as a deprecation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kitchen-inspec is a plugin for test-kitchen. It only works with VM's and containers. So it primarily needs ssh, winrm, and docker support. It's not running inspec profiles against an aws account, kubernetes, etc...
I've tested this and it's working. I also added the dokken tests to the GHA workflow for real integration tests with containers. Since kitchen-inspec is very limited to what it's working with to run inspec tests is why I switched this to inspec-core since it doesn't need all those extra features that aren't used.
.rubocop.yml
Outdated
| @@ -0,0 +1,8 @@ | |||
| --- | |||
| AllCops: | |||
| TargetRubyVersion: 3.4 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we be using chefstyle?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/inspec/kitchen-inspec/pull/315/files#diff-5154ee097a7705c7b7063dd7a8b640d2c5b6ef6dcd4a2f4868b71674289db618R18 your required ruby version is way lower than the targeted ruby version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chefstyle is dead and cookstyle is it's replacement. Cookstyle was extended to add --chefstyle flag to enable the same cops and settings chefstyle was using.
The targeted ruby version is for a more modern version for cookstyle but none of the changes introduced breaks functionality for older ruby versions so the gemspec ruby version is smaller. Modern rubocop versions also don't support anything less then 2.7.
That said if you think it's best to lower the target version I'm open to doing that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdyt of removing the target version completely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do that. looking up what rubocop defaults to it'll then look at the gemspec and if it can't find anything then it will set it to 2.7. I think it makes sense to let it follow the gemspec required_ruby_version.
Signed-off-by: Stromweld <hemminger@hotmail.com>
Description
Check List