-
Notifications
You must be signed in to change notification settings - Fork 543
feat: add binding metadata files plus fixes #3950
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 binding metadata files plus fixes #3950
Conversation
Signed-off-by: Samantha Coyle <[email protected]>
Signed-off-by: Samantha Coyle <[email protected]>
Signed-off-by: Samantha Coyle <[email protected]>
bindings/gcp/pubsub/pubsub.go
Outdated
| ClientID string `json:"clientID"` | ||
| AuthURI string `json:"authURI"` | ||
| TokenURI string `json:"tokenURI"` | ||
| AuthProviderX509CertURL string `json:"authProviderX509CertURL"` |
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.
This change is good because it consolidates our metadata, and is allowed since the binding is Alpha, but I am worried about breaking changes since I'm aware of several users using this binding in production
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 updated to use our mapstructurealiases in our kit decoder to support both approaches and added a unit test to double check :)
Signed-off-by: Samantha Coyle <[email protected]>
…m:sicoyle/components-contrib into feat-binding-component-registration-fixes
Signed-off-by: Samantha Coyle <[email protected]>
bindings/gcp/pubsub/pubsub.go
Outdated
| Topic string `json:"topic"` | ||
| Subscription string `json:"subscription"` | ||
| Type string `json:"type"` | ||
| ProjectID string `json:"projectID"` | ||
| PrivateKeyID string `json:"privateKeyID"` | ||
| PrivateKey string `json:"privateKey"` | ||
| ClientEmail string `json:"clientEmail"` | ||
| ClientID string `json:"clientID"` | ||
| AuthURI string `json:"authURI"` | ||
| TokenURI string `json:"tokenURI"` | ||
| AuthProviderX509CertURL string `json:"authProviderX509CertURL"` | ||
| ClientX509CertURL string `json:"clientX509CertURL"` | ||
| } |
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.
there are more changes here to align this alpha bindings component to what the stable pubsub version of it has for the auth profile so I could just use the builtinAuthenticationProfiles for gcp on its metadata yaml file
bindings/gcp/pubsub/pubsub.go
Outdated
| ClientID string `json:"clientID"` | ||
| AuthURI string `json:"authURI"` | ||
| TokenURI string `json:"tokenURI"` | ||
| AuthProviderX509CertURL string `json:"authProviderX509CertURL"` |
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 updated to use our mapstructurealiases in our kit decoder to support both approaches and added a unit test to double check :)
Signed-off-by: Samantha Coyle <[email protected]>
Signed-off-by: Samantha Coyle <[email protected]>
Signed-off-by: Samantha Coyle <[email protected]>
|
this is ready pls |
Description
Separating the bindings changes from PR:
#3949
For each dapr release there is an endgame task to ensure that components are properly registered across contrib and dapr/dapr. I took the time to fix all the issues that I came across, cross referencing the docs. This PR updates naming conventions on file directories, adds missing metadata.yaml files, and updates things like the gcp pubsub binding (alpha) to support the same logic as the stable pubsub gcp pubsub component so I could standardize their auth profiles in adding the metadata.yaml file and use the builtinAuthProfile for gcp.
You will also see TODO comments here and there. That is from me going through the code to manually create the metadata.yaml files and add todos i deemed necessary for the component. I also updated any
interface{}toanythat I came across along the way.RethinkDB needed custom changes to allow users to properly specify tlsconfig due to rethinkdb embedded structs/tags.
Yes, I did cross ref the docs too so the links should be 99.9% correct unless I made a typo or missed something accidentally.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
Note: We expect contributors to open a corresponding documentation PR in the dapr/docs repository. As the implementer, you are the best person to document your work! Implementation PRs will not be merged until the documentation PR is opened and ready for review.