Add support for default secret service and config

[jasonwbarnett]

Description

This gives users the ability to set a default secret service and secret service configuration

Initially I wrote no tests. I want to get in sync on the high level API before diving in to testing.

Before

value1 = secret(name: "test1", service: :aws_secrets_manager, config: { region: "us-west-1" })
value2 = secret(name: "test2", service: :aws_secrets_manager, config: { region: "us-west-1" })
value3 = secret(name: "test3", service: :aws_secrets_manager, config: { region: "us-west-1" })

After

default_secret_service(:aws_secrets_manager)
default_secret_config(region: "us-west-1")

value1 = secret(name: "test1")
value2 = secret(name: "test2")
value3 = secret(name: "test3")

# OR...

with_secret_service(:aws_secrets_manager) do
  with_secret_config(region: "us-west-1") do
    value1 = secret(name: "test1")
    value2 = secret(name: "test2")
    value3 = secret(name: "test3")
  end
end

Azure use case

You have multiple managed identities used to access different azure key vaults.

default_secret_service(:azure_key_vault)

with_secret_config(client_id: "3d5e5f4e-aaac-44e7-bb0d-140dfe69d6d8") do
  db_password = secret(name: "db_password")
  my_other_secret = secret(name: "my_other_secret")
end

with_secret_config(client_id: "c3874539-1bd6-49ea-a606-4637ce871690") do
  redis_password = secret(name: "redis_password")
  fun_secret = secret(name: "fun_secret")
end

Related Issue

N/A

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.
• All new and existing tests passed.
• All commits have been signed-off for the Developer Certificate of Origin.

[jasonwbarnett]

@marcparadise what do you think?

p.s. good design on your initial cut! How do I know that? I could easily add a new feature without changing any of the pre-existing code. 👍

[lamont-granquist]

we could support blocks to do this temporarily and then restore the old settings, that would look like this:

value1 = value2 = value3 = nil

default_secret_service :aws_secrets_manager do
  default_secret_config region: "us-west-1" do

    value1 = secret(name: "test1", version: "v1")
    value2 = secret(name: "test2", version: "v1")
    value3 = secret(name: "test3", version: "v1")
  end
end

i am... not entirely wild about this actually.

[marcparadise]

This looks good, other than our conversation in review about moving the config into the run context so that it's not persisted globally.

The way you've written this will make it pretty easy to take on the next step too, where we allow default secret config in Chef::Config.

[tas50]

I'm not opposed to this one, but I would like to hold off on merging it until we can have some conversations with customers around how they plan to use this resource. We get one take at how we define the defaults and we want to make sure we get it right.

[jasonwbarnett]

we could support blocks to do this temporarily and then restore the old settings, that would look like this:

value1 = value2 = value3 = nil

default_secret_service :aws_secrets_manager do
  default_secret_config region: "us-west-1" do

    value1 = secret(name: "test1", version: "v1")
    value2 = secret(name: "test2", version: "v1")
    value3 = secret(name: "test3", version: "v1")
  end
end

i am... not entirely wild about this actually.

I just finished adding this in a separate commit. It's not exactly elegant but I think it at least captures the concept.

[jasonwbarnett]

Alright ya'll, this is ready to go when you are.

I would love for the Chef community to give any feedback regarding this PR.

[lamont-granquist]

@tas50 thoughts? this is still in a holding pattern.

[tas50]

Still awaiting feedback

[lamont-granquist]

@marcparadise this is another good one for you to look at

[kenmacleod]

As a customer I'm not sure how to get on the feedback list for this feature. We're +1 on this PR.

In our case we've had a Chef Vault wrapper from the beginning decrypt_secret(SECRETGROUP, SECRETNAME) with the desire it would be future-compatible to migrating to AWS or Azure secrets. With this PR we should be able to keep the decrypt_secret() function and our teams would only need to provide the default config this PR provides.

Our typical case is several to dozens of secrets used by one team. Our desired outcome would be able to document a migration path where we tell the team to add the new initialization call (this PR) and existing decrypt_secret() calls would use the Secrets Helper to fetch the secrets.

This PR would mean we wouldn't need to manage provider defaults in our decrypt_secret() function.

[jasonwbarnett]

I just rebased onto the main branch

[johnmccrae]

Hi, can you please rebase off master again, we just corrected some problems with the Windows kitchen test setup files. Look for changes in .github/workflows/kitchen.yml that move the ansidecl.h file to it's correct location. The Ubuntu test looks like it was a timeout issue. This should run now.

[jasonwbarnett]

Hi, can you please rebase off master again, we just corrected some problems with the Windows kitchen test setup files. Look for changes in .github/workflows/kitchen.yml that move the ansidecl.h file to it's correct location. The Ubuntu test looks like it was a timeout issue. This should run now.

@johnmccrae just rebased.

[johnmccrae]

Waiting on a quick review from Marc Paradise before I merge this.

[marcparadise]

This looks good, the only thing I'm wondering is if we need with_secrets_config? I remember @lamont-granquist raised the possibility of adding this in an earlier review, but we talked through it after and were not sure that there is a valid use case for it.

[marcparadise]

Look

[lamont-granquist]

yeah i'm good not having that at least for the first cut.