Enable kitchen tests under Github Actions

[neha-p6]

Description

Chef-19 has introduced licensing so kitchen tests need some changes. This is phase 1 of those changes.

1. There is a forked version of test-kitchen now available chef-test-kitchen-enterprise which is a habitat package that can handle licensing. But the RC1 release of it currently supports kitchen-dokken driver on Linux x86_64 platforms only. So we will use it only for docker testing on linux machines under Github Actions (GHA)

2. To continue doing end-to-end recipe testing of Chef, for now on rest of the platforms kitchen testing will be done using legacy test-kitchen(corresponding jobs are named as legacy to signify the same) so that the code under a PR continues to get recipe tested.
   In future when chef-test-kitchen-enterprise will add support for new drivers and platforms, things will be modified under GHA to incorporate them.

3. Renamed kitchen-fips workflow as windows-fips as it does not use test-kitchen in any form but is only to test FIPS enablement on windows. The name causes confusion about its function.

4. There are few resources with a property's default value pointing to binary path of Infra client's executable. Since Chef 19 is now habitat executable instead of omnibus, all those properties and also rspec tests are now fixed here to point to habitat binary path of Infra Client.

Related Issue

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

• I have read the CONTRIBUTING document.
• I have run the pre-merge tests locally and they pass.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
• All new and existing tests passed.
• All commits have been signed-off for the Developer Certificate of Origin.

[neha-p6]

appbundle-updater integration is pending with chef-test-kitchen-enterprise. Work to make it habitat compliant is in progress.

[Stromweld]

I don't think we should rename the kitchen.*.yml files in the repo as they are not legacy they'd be the same for both community test-kitchen and test-kitchen-enterprise. Enterprise just adding support for additional config options for licensing.

[Stromweld]

Without app bundler to update code to latest PR committed then we are not testing new code changes before merging. This would break the meaning of doing these tests and thus this shouldn't be merged until that issue is resolved.

[rahulgoel1]

Without app bundler to update code to latest PR committed then we are not testing new code changes before merging. This would break the meaning of doing these tests and thus this shouldn't be merged until that issue is resolved.

yes appbundle-updater work is in progress and this will not be merged till that is resolved. Agree on it.

[neha-p6]

I don't think we should rename the kitchen.*.yml files in the repo as they are not legacy they'd be the same for both community test-kitchen and test-kitchen-enterprise. Enterprise just adding support for additional config options for licensing.

@Stromweld We had reached a conclusion to go this way in internal discussion reason being, with all the changes which we will have to do in future in those configuration yml files (eg, remove the omnitruck installations, other configurations specific omnibus installation path), each of those files are pretty much going to get changed, nothing apart from the VM boxes names is going to get reused. So we decided we will rename those as *legacy so that in future they can just be purged out and new habitat specific configuration files will be added replacing those (once platform support for other drivers and platforms is available)

[rahulgoel1]

I don't think we should rename the kitchen.*.yml files in the repo as they are not legacy they'd be the same for both community test-kitchen and test-kitchen-enterprise. Enterprise just adding support for additional config options for licensing.

@Stromweld We had reached a conclusion to go this way in internal discussion reason being, with all the changes which we will have to do in future in those configuration yml files (eg, remove the omnitruck installations, other configurations specific omnibus installation path), each of those files are pretty much going to get changed, nothing apart from the VM boxes names is going to get reused. So we decided we will rename those as *legacy so that in future they can just be purged out and new habitat specific configuration files will be added replacing those (once platform support for other drivers and platforms is available)

@neha-p6 @tpowell-progress @Stromweld I believe this requires additional discussion. While i agree we may have to run those in parallel however we will have regroup on how this will be from long term perspective. I still think naming can be done differently and exclude legacy to be able to proceed. We will try to have a discussion on this when we regroup on the review.

[Stromweld]

@rahulgoel1 just for clarity renaming the Github actions jobs is fine and makes sense the actual kitchen.yml files though are simply a configuration file test-kitchen reads in to know what to do. Those wont change from community to enterprise additions of test-kitchen except to add parameter for provisioner to add the license key for chef-client. All the other changes @neha-p6 is talking about would be done in code for the infra-client provisioner. Not in the kitchen.yml file that lives in cookbooks. You also can't remove all of the omnibus stuff or you break functionality for chef-client 18 and older versions of chef-client testing.

[tpowell-progress]

@neha-p6 moving to draft because based on several of our understanding, this should be merged to a "feature" branch for the moment (chef-19?)

[neha-p6]

@neha-p6 moving to draft because based on several of our understanding, this should be merged to a "feature" branch for the moment (chef-19?)

Yes sounds good. I'm going to maintain this branch itself as the feature branch, with any sub-task work being done on branches off of this branch as the base branch.

[neha-p6]

@rahulgoel1 just for clarity renaming the Github actions jobs is fine and makes sense the actual kitchen.yml files though are simply a configuration file test-kitchen reads in to know what to do. Those wont change from community to enterprise additions of test-kitchen except to add parameter for provisioner to add the license key for chef-client. All the other changes @neha-p6 is talking about would be done in code for the infra-client provisioner. Not in the kitchen.yml file that lives in cookbooks. You also can't remove all of the omnibus stuff or you break functionality for chef-client 18 and older versions of chef-client testing.

@Stromweld None of this is going to be backported to previous chef version branches if that's what we are thinking. Chef18 and previous versions are going to continue to use the community test-kitchen.
This entire task is Chef19 specific and reason for it is the licensing introduced by it. Since test-kitchen-enterprise currently supports kitchen-dokken driver and x86_64Linux platforms only, we are testing with community test-kitchen on rest of the platforms instead of removing the kitchen testing for them altogether (something is better than nothing!). And in future as test-kitchen-enterprise has support for more platforms and drivers we will drop using community test-kitchen tests, again this will apply only to chef19, nothing will be backported. I hope that clears the air.

[Stromweld]

@neha-p6 I understand that this is only for 19+ and not for backporting to chef-18. I think our wires are getting crossed here somewhere. I'll schedule a quick meeting for tomorrow to ensure we are all on the same page and understanding each other.

[tpowell-progress]

@neha-p6 it looks like both the front of the PATH variable and the GEM_PATH should reflect your hab pkg exec environment. GEM_PATH for my 18.7.6 install is /hab/pkgs/chef/chef-infra-client/18.7.6/20250421213716/vendor, so it looks like the current package info can be built dependably on that?

[jaymzh]

OK, so we discussed this at length in the meeting. The right call is most likely: File.realpath($PROGRAM_NAME).

If you use GEM_PATH or PATH, you have to make guesses, which is going to cause real issues.

If you use File.realpath($PROGRAM_NAME) you will definitively get the chef-client that was called, and that is the only sensible default for something like this resource.

There is one case where this won't work and that is if someone called chef-apply or chef-shell. In that case, we can either (1) call File.dirname and then tack on chef-client to the end or (2) throw an exception. I'm partial to an exception in this case, because chef-apply to call chef_client_cron with a default is a bit weird, but I don't have any objections to the first option.

So basically we're looking at code like:

def hab_chef_client_binary
  path = File.realpath($PROGRAM_NAME)
  bin = File.basename(path)
  return path if bin == 'chef-client'

  # this chunk optional...
  dir = File.dirname(path)
  probably = File.join(dir, 'chef-client')
  return probably if File.exist?(probably)

  fail "Cannot determine our chef-client binary"
end

[neha-p6]

@jaymzh @tpowell-progress After rebasing with main, which pulls in #14911, I'm seeing NameError: uninitialized constant Archive::EXTRACT_OWNER error on all the kitchen docker images (e.g almalinux-8)
We had been seeing this issue intermittently in the past and to get past had to pin ffi < 1.17.0 but since it worked in your pr without that pin, what else might be going here, any thoughts?

[jaymzh]

Well, since this is only happening on the new modified hab-based docker tests in this PR, and everything else works fine, my guess is it's something amis with the hab builds. This feels very Habitat-specific. Paging @mwrock ...

[mwrock]

I just submitted #15038 which I think may fix this.

[jaymzh]

I merged Matt's PR and rebased this, it should solve that issue.

[jaymzh]

So I mentioned this to @mwrock in slack, but based on what I'm seeing in the tests, it looks like this PR may be fundamentally flawed. I'm not sure, so if I'm misunderstanding @neha-p6 please clarify.

This PR seems to be installing previously-built chef-infra habitat packages. This means that when it is running on a PR it is in fact not testing the code in the PR.

The current tests on main test the code in the PR, which prevents bugs in the PRs from getting to main, but this would not. In fact, this would only catch errors from several PRs prior.

If I'm correct, that would be a severe regression in the testing, and we should definitely not move in this direction.

If the goal is to move kitchen testing to habitat packages, then the testing infra needs to build an adhoc hab package and use it. Doing that on each Linux platform is obviously a bit silly, but would suffice. Better would be to have an earlier workflow build and stash the package, later platforms depend on that, and pull the package.

Either way, it doesn't make sense regress the tests in a way that mean the PR the tests are running against not be what we're actually testing.

[neha-p6]

So I mentioned this to @mwrock in slack, but based on what I'm seeing in the tests, it looks like this PR may be fundamentally flawed. I'm not sure, so if I'm misunderstanding @neha-p6 please clarify.

This PR seems to be installing previously-built chef-infra habitat packages. This means that when it is running on a PR it is in fact not testing the code in the PR.

The current tests on main test the code in the PR, which prevents bugs in the PRs from getting to main, but this would not. In fact, this would only catch errors from several PRs prior.

If I'm correct, that would be a severe regression in the testing, and we should definitely not move in this direction.

If the goal is to move kitchen testing to habitat packages, then the testing infra needs to build an adhoc hab package and use it. Doing that on each Linux platform is obviously a bit silly, but would suffice. Better would be to have an earlier workflow build and stash the package, later platforms depend on that, and pull the package.

Either way, it doesn't make sense regress the tests in a way that mean the PR the tests are running against not be what we're actually testing.

@jaymzh It is testing the code in the inside the PR by rebasing the commit on top of previously built habitat package installation using appbundle-updater, just like how it's been done using existing community test kitchen for the omnibus installation of infra client. Ref https://github.com/chef/chef/pull/14794/files#diff-258802cb60657b6403c6aa88fe4c6b4406bd84f0ebc776c1c8fe0701333255f4R44-R65
For confirmation if you check the log of any of docker run which is using test-kitchen-enterprise using habitat installation of infra client, you'll see the Chef client version being updated to version inside the commit SHA.

e.g Version installed from test-kitchen-enterprise, which is the last RC release:

After updating binstubs and code to the commit SHA using appbundle-updater:

[jaymzh]

Ah, I see. Thanks for explaining. Assuming appbundler-updater works properly in habitat packages, that should be fine, but , you'll need to build a new hab package (however that happens), to get a usable hab packages for this PR to work correctly on.

[neha-p6]

Ah, I see. Thanks for explaining. Assuming appbundler-updater works properly in habitat packages, that should be fine, but , you'll need to build a new hab package (however that happens), to get a usable hab packages for this PR to work correctly on.

We have already made appbundle-updater habitat compliant so that a new package does not have to be built for testing each commit in a PR in kitchen tests which is time consuming. It updates the habitat dependencies just like how it used to do for omnibus. The appbundle-updater version 19.0.2.rc1 is the version which is being used here for habitat based package updates.

[jaymzh]

Yes, I understand that. However, you need a version that has Matt's build fixes in it to start from. That's not Ruby code so it won't be affected by appbundler-updater.

[neha-p6]

Yes, I understand that. However, you need a version that has Matt's build fixes in it to start from. That's not Ruby code so it won't be affected by appbundler-updater.

Ah I see. I thought you were referring to incorrect package being testing this PR in general.
Yeah since Matt's fix in habitat package's plan file, I'll need test-kitchen-enterprise to get a new package.

[Stromweld]

This can be simplified to:

Suggested change

	if os.windows?
	openssl_paths = Dir.glob("C:/hab/pkgs/core/openssl/*/*/bin/openssl.exe").sort
	openssl_bin = openssl_paths.last
	ca_file = "C:\\ssl_test\\my_ca.crt"
	cert_file = "C:\\ssl_test\\my_signed_cert.crt"
	# Currently community test kitchen is still used to run kitchen tests on windows, so we need to use omnibus path there
	# We will to do this until test-kitchen-enterprise supports other platforms and drivers.
	if !openssl_bin || !File.exist?(openssl_bin)
	openssl_bin = "C:\\opscode\\chef\\embedded\\bin\\openssl.exe"
	end
	else
	openssl_paths = Dir.glob("/hab/pkgs/core/openssl/*/*/bin/openssl").sort
	openssl_bin = openssl_paths.last
	ca_file = "/etc/ssl_test/my_ca.crt"
	cert_file = "/etc/ssl_test/my_signed_cert.crt"
	# Currently community test kitchen is still used to run kitchen tests on linux VMs, so we need to use omnibus path there
	# We will to do this until test-kitchen-enterprise supports other platforms and drivers.
	if !openssl_bin || !File.exist?(openssl_bin)
	openssl_bin = "/opt/chef/embedded/bin/openssl"
	end
	end
	cmd = "#{openssl_bin} verify -CAfile #{ca_file} #{cert_file}"
	describe command(cmd) do
	seperator = File::ALT_SEPARATOR || File::SEPARATOR
	openssl_path = File.join("#{os.windows? ? 'C:/hab' : '/hab'}", "pkgs", "core", "openssl", "*", "*", "bin", "openssl#{'.exe' if os.windows?}")
	openssl_bin = Dir.glob(openssl_path).sort.last.split("/").join(seperator)
	ca_file = ["#{os.windows? ? 'C:' : '/etc'}", 'ssl_test', 'my_ca.crt'].join(seperator)
	cert_file = ["#{os.windows? ? 'C:' : '/etc'}", 'ssl_test', 'my_signed_cert.crt'].join(seperator)
	# Currently community test-kitchen is still used to run kitchen tests on VMs, so we need to use omnibus path there
	# We will need to do this until test-kitchen-enterprise supports other platforms and drivers
	unless File.exist?(openssl_bin)
	openssl_bin = ["#{os.windows? ? 'C:\\opscode' : '/opt'}", 'chef', 'embedded', 'bin', "openssl#{'.exe' if os.windows?}"].join(seperator)
	end
	describe command("#{openssl_bin} verify -CAfile #{ca_file} #{cert_file}") do

[Stromweld]

I tested this to work on both windows and linux

[Stromweld]

@neha-p6 when troubleshooting that openssl_bin issue I found puts statements don't get printed but if you change it to raise you can get the output in the exception that gets created when testing. The suggested update I have above I did test it to work on both windows and linux VMs.

[Stromweld]

we can't use Github Secrets as this breaks community forks and PR jobs fail for them as they don't have access to our secrets. This is the same reason why we had to hard code a chef-client free license into kitchen.yml files vs using github actions secrets.

[jaymzh]

You can sometimes, fwiw. LIke this will work:

    name: DCO Check
    steps:
    - name: Get PR Commits
      uses: tim-actions/get-pr-commits@master
      id: 'get-pr-commits'
      with:
        token: ${{ secrets.GITHUB_TOKEN }}

I'm a bit unclear when it does and does not work though.

[Stromweld]

GITHUB_TOKEN only works as that's autogenerated for the specific GHA workflow action that is running and has limited access. Custom secrets such as this one needed to connect to public habitat builder to download the chef-test-kitchen-enterprise habitat package doesn't work with forks as that'd be a security issue for the secret.

[jaymzh]

Ah indeed. Yeah, that's right - static secrets vs session secrets.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.0% Duplication on New Code

See analysis details on SonarQube Cloud

[tpowell-progress]

@neha-p6 closing as assuming this is superseded by recent work