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.

Sign up for GitHub

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

Jump to bottom

Configure apprunner group, appstream group and add examples #144

Merged
merged 26 commits into from
Nov 21, 2022
Merged

Configure apprunner group, appstream group and add examples #144

merged 26 commits into from
Nov 21, 2022

Conversation

turkenf
Copy link
Collaborator

@turkenf turkenf commented Nov 14, 2022 •
edited
Loading

Description of your changes

This PR configures resources in the apprunner group and appstream group:

Apprunner group:

Appstream group:

Fixes: #10

Note: I couldn't configure aws_apprunner_custom_domain_association I reported an issue for this: #138

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

aws_apprunner_auto_scaling_configuration_version: https://github.com/upbound/provider-aws/actions/runs/3481806772
aws_apprunner_connection: https://github.com/upbound/provider-aws/actions/runs/3481840093
aws_apprunner_service: https://github.com/upbound/provider-aws/actions/runs/3481882734
aws_apprunner_vpc_connector: https://github.com/upbound/provider-aws/actions/runs/3481915083
aws_appstream_directory_config: https://github.com/upbound/provider-aws/actions/runs/3481927600
aws_appstream_fleet: https://github.com/upbound/provider-aws/actions/runs/3481969926
aws_appstream_fleet_stack_association: https://github.com/upbound/provider-aws/actions/runs/3486771048
aws_appstream_image_builder: https://github.com/upbound/provider-aws/actions/runs/3482919880
aws_appstream_stack: https://github.com/upbound/provider-aws/actions/runs/3486287692
aws_appstream_user: https://github.com/upbound/provider-aws/actions/runs/3482964886
aws_appstream_user_stack_association: https://github.com/upbound/provider-aws/actions/runs/3514011471

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 14, 2022

/test-examples="examples/apprunner/autoscalingconfigurationversion.yaml,examples/apprunner/connection.yaml,examples/apprunner/service.yaml,examples/apprunner/vpcconnector.yaml,examples/appstream/directoryconfig.yaml,examples/appstream/fleet.yaml,examples/appstream/fleetstackassociation.yaml,examples/appstream/imagebuilder.yaml,examples/appstream/stack.yaml,examples/appstream/user.yaml,examples/appstream/userstackassociation.yaml"

turkenh
turkenh reviewed Nov 14, 2022
View reviewed changes
config/apprunner/config.go Outdated Show resolved Hide resolved
@turkenf turkenf force-pushed the apprunner-appstream branch 2 times, most recently from dae3920 to 5c1f6a3 Compare November 14, 2022 12:27
@turkenf
Copy link
Collaborator Author

turkenf commented Nov 14, 2022

/test-examples="examples/apprunner/autoscalingconfigurationversion.yaml,examples/apprunner/connection.yaml,examples/apprunner/service.yaml,examples/apprunner/vpcconnector.yaml,examples/appstream/directoryconfig.yaml,examples/appstream/fleet.yaml,examples/appstream/fleetstackassociation.yaml,examples/appstream/imagebuilder.yaml,examples/appstream/stack.yaml,examples/appstream/user.yaml,examples/appstream/userstackassociation.yaml"

1 similar comment
@turkenf
Copy link
Collaborator Author

turkenf commented Nov 14, 2022

/test-examples="examples/apprunner/autoscalingconfigurationversion.yaml,examples/apprunner/connection.yaml,examples/apprunner/service.yaml,examples/apprunner/vpcconnector.yaml,examples/appstream/directoryconfig.yaml,examples/appstream/fleet.yaml,examples/appstream/fleetstackassociation.yaml,examples/appstream/imagebuilder.yaml,examples/appstream/stack.yaml,examples/appstream/user.yaml,examples/appstream/userstackassociation.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 14, 2022

/test-examples="examples/appstream/fleetstackassociation.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 14, 2022

/test-examples="examples/apprunner/autoscalingconfigurationversion.yaml,examples/apprunner/connection.yaml,examples/apprunner/service.yaml,examples/apprunner/vpcconnector.yaml,examples/appstream/directoryconfig.yaml,examples/appstream/fleet.yaml,examples/appstream/fleetstackassociation.yaml,examples/appstream/imagebuilder.yaml,examples/appstream/stack.yaml,examples/appstream/user.yaml,examples/appstream/userstackassociation.yaml"

ulucinar
ulucinar reviewed Nov 16, 2022
View reviewed changes
Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

Hi @turkenf,
Thank you for the PR!
Are the newly introduced example manifests testable on their own as a unit? Are they self-contained. i.e., each resource has its dependencies in its example manifest?

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

@ulucinar thanks for the review, each example has its own dependencies and can be tested.

ulucinar
ulucinar reviewed Nov 16, 2022
View reviewed changes
Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

Hi @turkenf,
I've left some API normalization comments. Could you please check them? And could you please apply similar normalization changes to the other resources as well (if needed)?

Please also don't forget to retest the resources with configuration changes with uptest. A mapping from Terraform resource names to the uptest runs as in the description of PR #147 would be really helpful.

config/externalname.go Outdated Show resolved Hide resolved
examples/appstream/user.yaml Outdated Show resolved Hide resolved
examples/appstream/user.yaml Show resolved Hide resolved
@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/appstream/user.yaml,examples/appstream/userstackassociation.yaml"

@turkenf
Generate with aws_apprunner_connection and copy examples from generated
e7810ae
@turkenf
Fix example for aws_apprunner_connection
6ad6e3c
@turkenf
Generate with aws_apprunner_auto_scaling_configuration_version and co…
98013b1 
…py examples from generated
@turkenf
Fix example for aws_apprunner_auto_scaling_configuration_version
c67880b
@turkenf
Generate with aws_apprunner_vpc_connector and copy examples from gene…
ac0175e 
…rated
@turkenf
Add reference config and fix example for aws_apprunner_vpc_connector
83a267b
@turkenf
Generate with aws_apprunner_service and copy examples from generated
c902165
@turkenf
Fix example for aws_apprunner_service
0da9178
@turkenf
Generate with aws_appstream_fleet and copy examples from generated
c1252e7
@turkenf
Add reference config and fix example for aws_appstream_fleet
b361f99
@turkenf
Generate with aws_appstream_directory_config and copy examples from g…
5a0d4b4 
…enerated
@turkenf
Fix example for aws_appstream_directory_config
b28264d
@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/apprunner/autoscalingconfigurationversion.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/apprunner/connection.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/apprunner/service.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/apprunner/vpcconnector.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/appstream/directoryconfig.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/appstream/fleet.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/appstream/fleetstackassociation.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/appstream/imagebuilder.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/appstream/stack.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/appstream/user.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 16, 2022

/test-examples="examples/appstream/userstackassociation.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 17, 2022

/test-examples="examples/appstream/stack.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 17, 2022

/test-examples="examples/appstream/fleetstackassociation.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 18, 2022

/test-examples="examples/appstream/userstackassociation.yaml"

1 similar comment
@turkenf
Copy link
Collaborator Author

turkenf commented Nov 18, 2022

/test-examples="examples/appstream/userstackassociation.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 18, 2022

/test-examples="examples/appstream/userstackassociation.yaml"

@turkenh turkenh mentioned this pull request Nov 21, 2022
Merged
3 tasks
@turkenf
Copy link
Collaborator Author

turkenf commented Nov 21, 2022

/test-examples="examples/appstream/userstackassociation.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 21, 2022

This PR is ready for review.

turkenh
turkenh approved these changes Nov 21, 2022
View reviewed changes
examples/apprunner/service.yaml
spec:
forProvider:
region: us-east-1
serviceName: example
Copy link
Contributor

Choose a reason for hiding this comment

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

Having serviceName as a parameter instead of using metadata.name does not look intuitive.

However, I believe there is no way to achieve this currently since the external name contains a randomly generated string by the provider. Crosslinking the related issue here: crossplane/upjet#142

@turkenh turkenh merged commit b560d85 into crossplane-contrib:main Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ulucinar ulucinar ulucinar left review comments

@turkenh turkenh turkenh approved these changes

@sergenyalcin sergenyalcin Awaiting requested review from sergenyalcin sergenyalcin is a code owner

@ezgidemirel ezgidemirel Awaiting requested review from ezgidemirel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Moving apprunner (5),appstream (7) resources to v1beta1 version
3 participants
@turkenf @ulucinar @turkenh