-
Notifications
You must be signed in to change notification settings - Fork 378
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 feature for RDS DB Parametergroups #577
Add feature for RDS DB Parametergroups #577
Conversation
Signed-off-by: Denis Holschuh <[email protected]>
Signed-off-by: Denis Holschuh <[email protected]>
Signed-off-by: Denis Holschuh <[email protected]>
a110739
to
8b8ae85
Compare
Signed-off-by: Denis Holschuh <[email protected]>
e90dbb5
to
4de48df
Compare
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
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.
Thanks @Dkaykay ! That's a pretty good start!
// | ||
// This value is stored as a lowercase string. | ||
// +kubebuilder:validation:Required | ||
DBParameterGroupName *string `json:"dbParameterGroupName"` |
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.
We are using external name annotation for this field, so we should ignore it in generator-config.yaml
like this. It needs to be ignored in all AWS calls in zz_controller.go
, i.e. CreateDBParameterGroupInput.DBParameterGroupName
etc.
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.
You need to re-run the code generator so that this field is removed.
…t to generator-config.yaml Signed-off-by: Dkaykay <[email protected]>
…payload obj Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
@muvaf can you review, please? Implemented & tested basic functionalities for creating, updating and deleting RDS DB Parametergroups |
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 a few small things but I think it's pretty close to merge. Thanks a lot @Dkaykay !
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
… more robustness Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
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! A few things:
- ACK needs to run again to remove the
DBParameterGroupName
from CRD. make reviewable
should work.- Could you squash the commits?
- Lastly, can you confirm given example YAML works? create, update, delete scenarios.
// | ||
// This value is stored as a lowercase string. | ||
// +kubebuilder:validation:Required | ||
DBParameterGroupName *string `json:"dbParameterGroupName"` |
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.
You need to re-run the code generator so that this field is removed.
examples/rds/dbparametergroup.yaml
Outdated
spec: | ||
forProvider: | ||
region: eu-central-1 | ||
dbParameterGroupName: example-dbparametergroup |
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.
dbParameterGroupName: example-dbparametergroup |
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 added to generator-config.yaml:
- CreateDBParameterGroupInput.DBParameterGroupName
- DeleteDBParameterGroupInput.DBParameterGroupName
- ModifyDBParameterGroupInput.DBParameterGroupName
- DescribeDBParameterGroupInput.DBParameterGroupName
But the field is not getting removed when running the generator. What am i missing here @muvaf ?
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.
Also, it seems that i am not authorized to squash & merge due to the branch being protected.
make reviewable test was green as well as create, update and delete scenarios.
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.
You can squash your commits locally and then git push --force
because you’re pushing to your own fork
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]> fix: remove DBParameterGroupName chore: make example more generic
…ykay/provider-aws into feature/rds-dbparametergroup
Signed-off-by: Denis Holschuh <[email protected]>
…ykay/provider-aws into feature/rds-dbparametergroup
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
…ykay/provider-aws into feature/rds-dbparametergroup
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
Signed-off-by: Dkaykay <[email protected]>
…ykay/provider-aws into feature/rds-dbparametergroup
closing PR and re-creating from non-broken branch |
…4.56.0 Bump Terraform provider version to v4.56.0
Description of your changes
Add rds/dbparametergroup resources as described in
#567
by following the guidelines in https://github.com/crossplane/provider-aws/blob/master/CODE_GENERATION.md
ToDo:
Add the possibility to pass in json / parameters that will be added/changed in the created DB parametergroup. It seems like these parameters cant be passed in at creation of the parametergroup. They need to be changed after it is created:
https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_ModifyDBParameterGroup.html