Feature/scaleway provider

[azert9]

Scaleway has recently made available a Secret Manager service. This pull requests adds support for it through a new scaleway provider.

Problem

n/a

Related Issue

Fixes #2081

Proposed Changes

How do you like to solve the issue and why?

Checklist

• I have read the contribution guidelines
• All commits are signed with git commit --signoff
• My changes have reasonable test coverage
• All tests pass with make test
• I ensured my PR is ready for review with make reviewable

[paul-the-alien]

Greetings!
Thank you for contributing to this project!
If this is your first time contributing, please make
sure to read the Developer and Contributing Process guides.
Please also mind and follow our Code of Conduct.

Useful commands:

• make fmt: Formats the code
• make check-diff: Ensures the branch is clean
• make reviewable: Ensures a PR is ready for review

[moolen]

I did a test drive with it, looks good overall, but there seems to be an issue with the name: based API, see below.

1. GET secret by name results in 404.

There seems to be a bug in the API. Requesting a secret via name: doesn't work. The API responds with a 404. I double-checked all parameters.

  data:
    - secretKey: example
      remoteRef:
        key: name:secret-sweet-archimedes

Accessing the secret via name doesn't work.
Also: it's weird that it is possible to have two secrets with the very same name in the the same region. Not sure if this is intended, but the this API call 👇 doesn't point to a uniquely identifiable resource.

curl -i -H "X-Auth-Token: xxxxxxxxx" https://api.scaleway.com/secret-manager/v1alpha1/regions/fr-par/secrets-by-name/secret-sweet-archimedes/versions/latest_enabled
{"message":"resource is not found","resource":"secret","resource_id":"secret-sweet-archimedes","type":"not_found"}

Accessing the secret directly via it's id works as expected:

curl -i -H "X-Auth-Token: xxxxxxxxx" https://api.scaleway.com/secret-manager/v1alpha1/regions/fr-par/secrets/a284d53b-1ea2-4cc5-9850-0b0a6d2ec490/versions/latest_enabled/access
{"secret_id":"a284d53b-1ea2-4cc5-9850-0b0a6d2ec490","revision":1,"data":"Zm9vYmFy"}

2. Push secret results in duplicates

Due to the error above 👆 we end up creating multiple secrets

Do you by chance know a way to provide feedback to scaleway for this beta API?

[moolen]

Do we really have to use name: here? if this is the only way to push secrets then i guess it can be omitted for brevity.

[azert9]

Wouldn't it be confusing to the user to have two different syntaxes for remote references?
We could default to name: everywhere, but it would be less clear that the two means of referencing secrets exist (the id is preferred when possible because it has no unicity issues).
What do you think?

[moolen]

Fair points!
Sorry for the wall of text, i just wanted to dump my thoughts here. Naming is hard 😂 😅

(1) fix name collision upstream (???)
IMHO both options to reference secrets in Scaleway SM are suboptimal:

1. id is hard to work with: hard to read, risk of human error
2. name is ambiguous, hence (in a strict sense) useless

My gut feeling tells me that this is a API design fault and i guess we're not the only ones suffering from that. I reckon there will/is a terraform provider out there that uses this API that suffers from that problem. Given that the API is in beta this is still fixable i hope. Honestly i don't have the time nor the network to start a discussion with this vendor. Maybe you have or already know the answer that this is a WONTFIX 🤔.

(2) remoteRef.key syntax
Moving forward, i agree, we should be explicit with the syntax for the remote reference.
For context: Azure KeyVault has different object types you can store: key, cert, secret and their remote reference contains this type in the remoteRef.key and uses this schema: secret/{name}, key/{name} etc.
Having and id: or name: prefix looks a bit odd (reading it as yaml string) and a bit inconsistent with the Azure KeyVault implementation.
Of course, semantically these are two different things: object-type vs reference-type. Hence a different syntax is appropriate.

Long-term it may be worth looking into standardising this into a RFC3986 URI compatible scheme, but that's a whole other story.

Simply put: yep, let's leave it as you have suggested 😁

[azert9]

I can confirm it will probably stay a WONTFIX... As you say naming is hard :)

[azert9]

Hello moolen, thanks for your feedback!

Regarding secret names the API is a little confusing but it was designed this way for coherence with other product APIs.

There is no uniqueness constraint on names, but if the user chooses to reference secrets by name, he must ensures uniqueness by himself (queries on a non-unique name result on an error).

The 404 error that you got was a bug, it should be fixed by now.

[moolen]

/ok-to-test sha=dd29cba

[moolen]

/ok-to-test sha=622ecc4

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[moolen]

Went for another round of testing, looks good to me! Thank you so much for also providing e2e tests, much appreciated!
I've added the necessary secrets to make it work. However, this needs to be merged before the tests become green. I'm gonna keep an eye on it.