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.

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

feat: add scaleway provider with scaleway_function_namespace resource support #1648

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

norbjd
Copy link

@norbjd norbjd commented Mar 27, 2023

Q A
πŸ› Bug fix? no
πŸš€ New feature? yes
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues
❓ Documentation yes

Description

Hello πŸ‘‹

First of all, thanks for this great tool! 😍

This PR adds support for Scaleway provider. Scaleway is an european cloud provider.

I've followed the documentation to add a new provider, and also added a new resource (for testing purpose): scaleway_function_namespace (a resource related to the serverless function product to logically group serverless functions). For reference, the description of scaleway_function_namespace is here, and the Terraform definition is here. I have added unit and acceptance tests to verify everything works properly.

If this PR is satisfying, I'll try to add more resources. It should be probably possible to add more resources automatically from the Go SDK or the Terraform provider.

I also have a few questions since it's my first PR here (I've added GitHub comments on the related code).

Thank you very much πŸ˜ƒ

@norbjd norbjd requested a review from a team as a code owner March 27, 2023 11:38
@norbjd norbjd requested review from wbeuil and removed request for a team March 27, 2023 11:38
@CLAassistant
Copy link

CLAassistant commented Mar 27, 2023

CLA assistant check
All committers have signed the CLA.

@@ -21,7 +21,7 @@

<p align="center">
Measures infrastructure as code coverage, and tracks infrastructure drift.<br>
<strong>IaC:</strong> Terraform. <strong>Cloud providers:</strong> AWS, GitHub, Azure, GCP.<br>
<strong>IaC:</strong> Terraform. <strong>Cloud providers:</strong> AWS, GitHub, Azure, GCP, Scaleway (alpha).<br>
Copy link
Author

Choose a reason for hiding this comment

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

I've added Scaleway in the list of providers here (but advertised it's still in alpha since only one resource is available). Tell me if we should wait before putting it in the README!

)

func InitResourcesMetadata(resourceSchemaRepository resource.SchemaRepositoryInterface) {
initScalewayFunctionNamespace(resourceSchemaRepository)
Copy link
Author

Choose a reason for hiding this comment

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

For other providers, is there a way to automatically generate that code? I feel like we could πŸ€”

@codecov-commenter
Copy link

Codecov Report

Merging #1648 (1d9632d) into main (50f8f2d) will decrease coverage by 1.59%.
The diff coverage is 75.00%.

πŸ“£ This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1648      +/-   ##
==========================================
- Coverage   79.60%   78.02%   -1.59%     
==========================================
  Files         229      224       -5     
  Lines        7183     7030     -153     
==========================================
- Hits         5718     5485     -233     
- Misses       1270     1347      +77     
- Partials      195      198       +3     
Impacted Files Coverage Ξ”
pkg/resource/resource_types.go 76.47% <ΓΈ> (ΓΈ)
pkg/resource/schemas/repository.go 63.88% <50.00%> (-0.54%) ⬇️
pkg/resource/scaleway/metadatas.go 100.00% <100.00%> (ΓΈ)
...g/resource/scaleway/scaleway_function_namespace.go 100.00% <100.00%> (ΓΈ)

... and 26 files with indirect coverage changes

scw.WithDefaultRegion(scw.RegionFrPar),
scw.WithDefaultZone(scw.ZoneFrPar1),
scw.WithUserAgent(fmt.Sprintf("driftctl/%s", version.Current())),
scw.WithProfile(profile),
Copy link
Author

Choose a reason for hiding this comment

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

If the profile contains a region (e.g. fr-par, nl-ams, pl-waw), the client will use this region instead of the default fr-par. Also, since one client can only process resources on one region, when listing resources (e.g. function namespaces), we'll only get resources from that specific region. I took a look at other providers like AWS, and it looks like it's the same behavior. Could you confirm? πŸ™

@norbjd
Copy link
Author

norbjd commented May 25, 2023

Hello @wbeuil πŸ‘‹ have you had the chance to take a look at this? Any feedback would be appreciated, thanks in advance! Also sorry for the ping, but I don't really know who else to tag πŸ˜…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants