-
Notifications
You must be signed in to change notification settings - Fork 383
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 Api Gateway RestAPI and Resource resources #1230
Add Api Gateway RestAPI and Resource resources #1230
Conversation
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 looks like a nice PR!
When running the examples, I get the following issue parsing the policy:
.648561326001369e+09 DEBUG provider-aws Cannot observe external resource {"controller": "managed/restapi.apigateway.aws.crossplane.io", "request": "/test", "uid": "e3724d47-b3c1-42ad-ba4c-b41c74cd46ff", "version": "237985", "external-name": "onnex4cv1b", "error": "late-init failed: policies couldnt be compared: cant parse policies: cant unmarshal policy: invalid character '\\\\' looking for beginning of object key string", "errorVerbose": "policies couldnt be compared: cant parse policies: cant unmarshal policy: invalid character '\\\\' looking for beginning of object key string\
{"object": {"kind":"RestAPI","name":"test","uid":"e3724d47-b3c1-42ad-ba4c-b41c74cd46ff","apiVersion":"apigateway.aws.crossplane.io/v1alpha1","resourceVersion":"237985"}, "reason": "CannotObserveExternalResource", "message": "late-init failed: policies couldnt be compared: cant parse policies: cant unmarshal policy: invalid character '\\\\' looking for beginning of object key string"}
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 need to dive deeper into the policy parsing, as I am getting errors in testing and there is some odd behavior about quoting.
I managed to fix the policy parsing. For an explanation on what's happening:
but post-creation, aws will return
This wasn't playing nice with json marshal/unmarshal. I found a solution that works here: https://stackoverflow.com/a/50151862. This substitutes the string replace approach. |
@tiagoposse Thanks! The new fixes look like they resolve the parsing issue. NAME READY SYNCED EXTERNAL-NAME
restapi.apigateway.aws.crossplane.io/test True True onnex4cv1b
NAME READY SYNCED EXTERNAL-NAME
resource.apigateway.aws.crossplane.io/test True True f2dndj |
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.
LGTM. Thanks for your contribution!
I have a small PR to your branch to fix a few errors. https://github.com/tiagoposse/provider-aws/pull/1
My pleasure, looking forward to open PRs for rest of the resources once this is merged :) |
@tiagoposse do you think you could rebase this PR on master and push your branch? There was a linter fix in #1250 that we think is affecting the checks on this pR. |
Signed-off-by: Tiago Posse <[email protected]>
Signed-off-by: Tiago Posse <[email protected]>
Signed-off-by: Tiago Posse <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
done! |
Doesn't seem to have fixed it, anything else I can do here ? |
I will pull your branch and see if there are any build issues in my environment. It's possible we still have issues in the GitHub action. |
Signed-off-by: Steven Borrelli <[email protected]>
@tiagoposse I think the PR got caught up in some changes in the upstream API. I've opened a PR against your repo that hopefully fixes the issues: https://github.com/tiagoposse/provider-aws/pull/2 |
…estapi update generated files and crds
Looks like all the checks have passed. We'll need one of the maintainers to approve and merge. |
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.
@stevendborrelli thanks for review testing and help with PRs ;) tested the resource also with success LGTM
* generate apigateway service resources * add resource and restapi apigateway resources Signed-off-by: Tiago Posse <[email protected]> Co-authored-by: Steven Borrelli <[email protected]> Signed-off-by: Felipe Barbosa <[email protected]>
Description of your changes
Generate resources for the apigateway service and add restapi and resource as usable.
First part of #1185
I have:
make reviewable test
to ensure this PR is ready for review.How has this code been tested
Tests and example yamls included.