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.

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 support for CloudSearch #1016

Merged
merged 2 commits into from Mar 8, 2022
Merged

Conversation

mhoff
Copy link
Contributor

@mhoff mhoff commented Dec 17, 2021

Description of your changes

Implements basic support for CloudSearch domains.
Some implementation details still up for discussion.

Fixes #1007.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

I will:

  • Write tests for the code in setup
  • Add examples

How has this code been tested

Simple create/delete/update procedures with test resources.
End-to-end testing from creating a domain up to using it to query data using curl and anonymous access (index fields not covered by this PR).

Discussion

The AWS API for CloudSearch does provide a lean data model for Domains (name, region, current scaling values, endpoints). Other parameters (like scaling options, access policy, availability options, index fields) are handled through separate Define/Describe functions, which therefore are excluded from the generated domain data model.
In this PR, I chose the approach to include some of those parameters in the custom domain parameters and to use (sometimes contextless) sub-calls to handle late-init, isUpToDate and update.

Copy link
Collaborator

@MisterMX MisterMX left a comment

Choose a reason for hiding this comment

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

Thanks @mhoff for your contribution! Good first PR.

I added some remarks to your code.

apis/cloudsearch/v1alpha1/generator-config.yaml Outdated Show resolved Hide resolved
pkg/controller/cloudsearch/domain/setup.go Outdated Show resolved Hide resolved
pkg/controller/cloudsearch/domain/setup.go Outdated Show resolved Hide resolved
pkg/controller/cloudsearch/domain/setup.go Outdated Show resolved Hide resolved
pkg/controller/cloudsearch/domain/setup.go Outdated Show resolved Hide resolved
pkg/controller/cloudsearch/domain/setup.go Outdated Show resolved Hide resolved
pkg/controller/cloudsearch/domain/setup.go Outdated Show resolved Hide resolved
pkg/controller/cloudsearch/domain/setup.go Outdated Show resolved Hide resolved
pkg/controller/cloudsearch/domain/setup.go Outdated Show resolved Hide resolved
@mhoff
Copy link
Contributor Author

mhoff commented Dec 20, 2021

Thanks @mhoff for your contribution! Good first PR.

I added some remarks to your code.

Many thanks for your remarks! I have resolved the few minor ones and will continue with the rest.

@mhoff mhoff force-pushed the cloudsearch branch 5 times, most recently from 88b599d to 4ada4a4 Compare December 22, 2021 09:04
@mhoff mhoff marked this pull request as ready for review December 22, 2021 09:04
@mhoff mhoff changed the title Add support for CloudSearch Draft: Add support for CloudSearch Feb 2, 2022
@mhoff mhoff changed the title Draft: Add support for CloudSearch Add support for CloudSearch Feb 7, 2022
@mhoff
Copy link
Contributor Author

mhoff commented Feb 7, 2022

@haarchri I have updated the PR according to what we have discussed. This PR should properly implement alpha-support for AWS CloudSearch. Review would be very much appreciated.

@mhoff mhoff force-pushed the cloudsearch branch 3 times, most recently from 42f0e71 to a29ad0b Compare February 10, 2022 09:48
Signed-off-by: Michael Hoff <michael.hoff-extern@deutschebahn.com>
(external expert on behalf of DB Netz AG)
…d metadata.name

Signed-off-by: haarchri <chhaar30@googlemail.com>
@haarchri
Copy link
Member

haarchri commented Mar 8, 2022

image

NAME                                                           READY   SYNCED   EXTERNAL-NAME
domain.cloudsearch.aws.crossplane.io/cloudsearch-test-anonym   True    True     cloudsearch-test-anonym
domain.cloudsearch.aws.crossplane.io/cloudsearch-test-domain   True    True     cloudsearch-test-domain

Copy link
Member

@haarchri haarchri left a comment

Choose a reason for hiding this comment

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

LGTM @mhoff thanks for contribution and all changes & rebases

@haarchri haarchri merged commit df4b03c into crossplane-contrib:master Mar 8, 2022
tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024
…ne-3.x

Update alpine Docker tag to v3.19.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for CloudSearch
3 participants