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.

Sign up for GitHub

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

Jump to bottom

feat: ibm provider implementation #235

Merged
merged 5 commits into from
Jul 5, 2021
Merged

feat: ibm provider implementation #235

merged 5 commits into from
Jul 5, 2021

Conversation

knelasevero
Copy link
Member

@knelasevero knelasevero commented Jun 29, 2021 •
edited

We worked on this one doing mob programming with some people at CS. Some tried Golang for the first time, some are even new to kubernetes. I think this is pretty cool 😄

This is just a basic GetSecret and GetSecretMap implementation. Still missing e2e tests and support for the property field.
Closes #66

@knelasevero knelasevero force-pushed the ibm_provider branch 4 times, most recently from 16baed2 to e8af112 Compare June 29, 2021 19:32
@knelasevero
Copy link
Member Author

/ok-to-test sha=e8af112

@knelasevero
Copy link
Member Author

Gonna add docs and e2e tests on another PR later

@knelasevero knelasevero requested a review from moolen June 30, 2021 12:38
@knelasevero
Copy link
Member Author

/ok-to-test sha=1427997

1 similar comment
@knelasevero
Copy link
Member Author

/ok-to-test sha=1427997

@knelasevero
Copy link
Member Author

Hmm 🤔 so when github actions is slow our e2e tests fail to start and gets timed out

@knelasevero
Copy link
Member Author

/ok-to-test sha=1427997

@knelasevero
Copy link
Member Author

/ok-to-test sha=67a1ba5

@knelasevero
Copy link
Member Author

@moolen where do we set this specific timeout? I saw in the code we setting timeouts for waiting for URLs and for waiting for responses, but not for the framework/setup to start.

@knelasevero
Copy link
Member Author

/ok-to-test sha=e8af112

@moolen
Copy link
Member

moolen commented Jul 2, 2021

/ok-to-test sha=1427997

@moolen
Copy link
Member

moolen commented Jul 2, 2021 •
edited

@moolen where do we set this specific timeout? I saw in the code we setting timeouts for waiting for URLs and for waiting for responses, but not for the framework/setup to start.

In the logs of this run i see:

external-secrets-control-plane   NotReady   control-plane,master   7m2s   v1.20.2   172.18.0.2    <none>        Ubuntu 20.10   5.8.0-1036-azure   containerd://1.4.0-106-gce4439a8
Granting permissions to e2e service account...
serviceaccount/external-secrets-e2e created
clusterrolebinding.rbac.authorization.k8s.io/permissive-binding created
Waiting service account...
customresourcedefinition.apiextensions.k8s.io/clustersecretstores.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/externalsecrets.external-secrets.io created
customresourcedefinition.apiextensions.k8s.io/secretstores.external-secrets.io created
Starting the e2e test pod
error: timed out waiting for the condition
pod "e2e" deleted
make[1]: *** [Makefile:20: test] Error 1
make[1]: Leaving directory '/home/runner/work/external-secrets/external-secrets/e2e'
make: *** [Makefile:87: test.e2e] Error 2

I would assume that we need to specify --pod-running-timeout=10m here (defaults to 1m): https://github.com/external-secrets/external-secrets/blob/main/e2e/run.sh#L52

Also, the kind node is NotReady in this test, it was ready previously.... weird 🤷‍♂️
Maybe a kubectl describe node will give us more insights in why the kind node ist not ready yet.

@knelasevero
Copy link
Member Author

/ok-to-test sha=82efb8e

@knelasevero
Copy link
Member Author

The NotReady did not catch my eye before 😂 I am gonna check it out

@knelasevero
Copy link
Member Author

/ok-to-test sha=ee50ee8

ricardotorresdacosta and others added 4 commits July 2, 2021 16:00
@ricardotorresdacosta @sebagomez
@knelasevero @Jabray5 @ElsaChelala @choilmto @amouat @ttarczynski
feat: ibm provider implementation
fd53e76 
Co-authored-by: Sebastián Gómez <sebastiangomezcorrea@gmail.com>
Co-authored-by: Lucas Severo <lucassalves65@gmail.com>
Co-authored-by: Joey Brayshaw <joeybrayshaw@gmail.com>
Co-authored-by: Elsa Chelala <elsachelala@gmail.com>
Co-authored-by: choilmto <choilmto@gmail.com>
Co-authored-by: Adrian Mouat <adrian.mouat@gmail.com>
Co-authored-by: ricardoptcosta <ricardoptcosta@gmail.com>
Co-authored-by: Gabi Beyer <Gabrielle.Beyer@container-solutions.com>
Co-authored-by: Tomasz Tarczynski <ttarczynski@users.noreply.github.com>
Co-authored-by: Mircea Cosbuc <mircea.cosbuc@container-solutions.com>
@knelasevero
docs: ibm provider
f97e4eb
@knelasevero
increase pod-running-timeout
6637f77
@knelasevero
fix: simpler create secret command
812ddf4
@knelasevero
Copy link
Member Author

/ok-to-test sha=812ddf4

@knelasevero
Copy link
Member Author

knelasevero commented Jul 2, 2021 •
edited

@moolen ok, now it worked. Upgraded kind and increased some timeouts: https://github.com/external-secrets/external-secrets/runs/2973310813?check_suite_focus=true

moolen
moolen reviewed Jul 5, 2021
View reviewed changes
pkg/provider/ibm/provider.go
}
response, _, err := ibm.IBMClient.GetSecret(
&sm.GetSecretOptions{
SecretType: core.StringPtr(sm.GetSecretOptionsSecretTypeArbitraryConst),
Copy link
Member

Choose a reason for hiding this comment

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

must the secret_type match with the secret id? E.g. what happens if we request type=arbitrary when the secret is actually a type=imported_cert ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It has to match. If we request a type=arbitrary and we have a type=imported_cert on the SM, it will fail with key \"x-x-x-x\" from ExternalSecret \"example\": Not Found". I would say we only support type=arbitrary for now.

I am going to update the docs.

@knelasevero knelasevero mentioned this pull request Jul 5, 2021
Closed
@knelasevero knelasevero force-pushed the ibm_provider branch 2 times, most recently from bd7c50d to 18b365b Compare July 5, 2021 10:10
moolen
moolen approved these changes Jul 5, 2021
View reviewed changes
Copy link
Member

@moolen moolen left a comment

Choose a reason for hiding this comment

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

Cool, Looks good to me! Lets get it in as it is RN and add the missing bits later!

@knelasevero
Copy link
Member Author

/merge

@paul-the-alien paul-the-alien bot merged commit abdfbac into external-secrets:main Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moolen moolen moolen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

IBM Cloud Secrets Manager
3 participants
@knelasevero @moolen @ricardotorresdacosta