Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Gitlab CI variables #319

Merged
merged 12 commits into from
Aug 26, 2021
Merged

Conversation

Jabray5
Copy link
Contributor

@Jabray5 Jabray5 commented Aug 11, 2021

Introduced Gitlab provider

Copy link
Contributor

@paul-the-alien paul-the-alien bot left a comment

Choose a reason for hiding this comment

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

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.

@knelasevero
Copy link
Member

Basically, unit tests and mocking missing, right? Need any specific help going over those?

@Jabray5
Copy link
Contributor Author

Jabray5 commented Aug 12, 2021

Basically, unit tests and mocking missing, right? Need any specific help going over those?

Yeah I didn't manage to quite figure out what the mock providers were doing. Any tips would be great!

@knelasevero
Copy link
Member

Basically, unit tests and mocking missing, right? Need any specific help going over those?

Yeah I didn't manage to quite figure out what the mock providers were doing. Any tips would be great!

So, we want a mock client struct that is similar to the real client struct that we imported in the real implementation.

If you look at GCP implementation, the line that actually gets a secret value is this:

result, err := sm.SecretManagerClient.AccessSecretVersion(ctx, req)

and there we see that the client struct needs a method named AccessSecretVersion (with the same signature and all that).

So in the fake client for gcp, we create a mock client struct that has a method like that.

func (mc *MockSMClient) AccessSecretVersion(ctx context.Context, req *secretmanagerpb.AccessSecretVersionRequest, opts ...grpc.CallOption) (*secretmanagerpb.AccessSecretVersionResponse, error) {

same thing for IBM, here you can see that IBM gets a secret in this line:

response, _, err := ibm.IBMClient.GetSecret(

And therefore we need our mock client struct to be similar to the real struct and to have a method named GetSecret (with the same signature and everything).

func (mc *IBMMockClient) GetSecret(getSecretOptions *sm.GetSecretOptions) (result *sm.GetSecret, response *core.DetailedResponse, err error) {

This is important, because when we call unit tests, we don't want the code to call the actual client, because we don't want to do real calls to the providers apis. Using IBM as an example, for unit testing, we want to test (normally run through real code) the provider going through lines 95 to just before 110, mock what is being returned from the client, and go normally until another call to the client, mock it, and so on, until the end of the method. We test the simple logic of the method, and not client API calls.

In the test setup we call

smtc.mockClient.WithValue(smtc.apiInput, smtc.apiOutput, smtc.apiErr)
the withValue method to overwrite the important method in the fake client to receive anything we want and return any arbritary data we want.

You can see the WithValue method overriding what getSecret does here WithValue. And GetSecret just calls getSecret.

And before looping over our testCases, we set the provider client to be the mock client instead of a real one. (In real life, the controller will know wich client to create and assign, if you wanna check that).

Here you can see the test setting the mock client before looping over tests:

sm.IBMClient = v.mockClient

In your case you will have to create a fake client struct that has a method named GetVariable (with same signature, same arguments and all that, similar to the client struct that you imported from the gilab official package). Then an indirection func (that you can call anything really) that your GetVariable just calls. And a WithValue that overrides this indirection func.

@knelasevero
Copy link
Member

BTW, great work making it already pass all e2e tests and be functional already! 🎉

@Jabray5 Jabray5 force-pushed the gitlab-ci-secrets branch from 7240b9a to 7334710 Compare August 24, 2021 12:07
@knelasevero
Copy link
Member

/ok-to-test sha=079e11d8a403b422d41115c37749dd7e090d843c

@knelasevero knelasevero force-pushed the gitlab-ci-secrets branch 4 times, most recently from fe4974f to 61d44b9 Compare August 24, 2021 21:30
@knelasevero knelasevero marked this pull request as ready for review August 24, 2021 21:30
@knelasevero
Copy link
Member

/approve

@knelasevero
Copy link
Member

Docs missing, but we can add docs later if you want :)

LGTM!

@knelasevero
Copy link
Member

/ok-to-test sha=ad966ea2efdd6c3bb5e87bd3709cbba7257f043f

@knelasevero
Copy link
Member

/merge

@paul-the-alien paul-the-alien bot merged commit 5e433b6 into external-secrets:main Aug 26, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants