-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add integration testing package #89
Conversation
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nitpicks and suggestions, but I'm happy to merge this whenever you feel it's ready.
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" | ||
"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" | ||
|
||
// Allow auth to cloud providers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened the https://godoc.org/k8s.io/client-go/plugin/pkg/client/auth trying to find more context on what it means to allow auth to cloud providers by importing a package, but could not find any. Perhaps it's worth adding a little more explanation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just allows you to authenticate to managed k8s offerings on a variety of providers. The GCP plugin has a pretty good doc here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good context, thanks! Would you mind capturing it in the comment in case any future readers are also unclear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still prefer a clearer explanation here, but will merge to unblock this work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops yes meant to add a more descriptive comment, will follow up!
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
@negz all changes have been addressed either with changes or comments, thanks for your feedback! RTM if looks good to you :) |
* crossplane#89 * https://github.com/search?q=%22github.com%2Fcrossplane%2Fcrossplane-runtime%2Fpkg%2Ftest%2Fintegration%22&type=code This was added in the above PR, but we never really made use of it. Based on the above GitHub search, I think every consumer of the package (except https://github.com/vshn/crossplane-service-broker?) are ancient, stale forks of kubevela, oam-kubernetes-runtime, or provider-gcp. Modern versions of these packages no longer use this one. If we wanted to be really conservative we could mark this deprecated, but I lean toward just removing it. Signed-off-by: Nic Cope <nicc@rk0n.org>
Description of your changes
Moves integration testing framework from https://github.com/hasheddan/athodyd. For example implementation see crossplane-contrib/provider-gcp#96.
Checklist
I have:
make reviewable
to ensure this PR is ready for review.clusterrole.yaml
to include any new types.