Add conversion functions for handling some breaking API changes by sergenyalcin · Pull Request #1118 · crossplane-contrib/provider-upjet-aws

sergenyalcin · 2024-01-31T11:52:43Z

Description of your changes

This PR adds,

v1beta2 versions of the following resources:
- aws_autoscaling_attachment
- aws_autoscaling_group
- aws_connect_hours_of_operation
- aws_connect_queue
- aws_connect_routing_profile
- aws_db_instance
- aws_elasticache_replication_group
- aws_msk_cluster
- aws_route
- aws_route_table
conversion functions for handling some breaking API changes
a new make target that calls a bash script to checkout to old commits the types.go files of older API versions
central injector for field renaming conversions. For following fields renaming, field renaming conversion functions are injected automatically:

    aws_autoscaling_attachment:
      spec.forProvider.albTargetGroupArn: spec.forProvider.lbTargetGroupArn
      spec.initProvider.albTargetGroupArn: spec.initProvider.lbTargetGroupArn
      status.atProvider.albTargetGroupArn: status.atProvider.lbTargetGroupArn
    aws_connect_hours_of_operation:
      status.atProvider.hoursOfOperationArn: status.atProvider.arn
    aws_connect_queue:
      status.atProvider.quickConnectIdsAssociated: status.atProvider.quickConnectIds
    aws_db_instance:
      spec.forProvider.name: spec.forProvider.dbName
      spec.initProvider.name: spec.initProvider.dbName
      status.atProvider.name: status.atProvider.dbName
    aws_elasticache_replication_group:
      spec.forProvider.numberCacheClusters: spec.forProvider.numCacheClusters
      spec.forProvider.replicationGroupDescription: spec.forProvider.description
      spec.forProvider.availabilityZones: spec.forProvider.preferredCacheClusterAzs
      spec.initProvider.numberCacheClusters: spec.initProvider.numCacheClusters
      spec.initProvider.replicationGroupDescription: spec.initProvider.description
      spec.initProvider.availabilityZones: spec.initProvider.preferredCacheClusterAzs
      status.atProvider.numberCacheClusters: status.atProvider.numCacheClusters
      status.atProvider.replicationGroupDescription: status.atProvider.description
      status.atProvider.availabilityZones: status.atProvider.preferredCacheClusterAzs
    aws_route:
      spec.forProvider.instanceId: spec.forProvider.networkInterfaceId
      spec.initProvider.instanceId: spec.initProvider.networkInterfaceId
    aws_route_table:
      status.atProvider.route.instanceId:  status.atProvider.route.networkInterfaceId

custom converter functions for the following resources:
- aws_autoscaling_group
- aws_connect_routing_profile
- aws_elasticache_replication_group
- aws_msk_cluster

I have:

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

How has this code been tested

For the following resources the spec.forProvider and status.atProvider was tested. While testing, checked two versions of resources.

aws_autoscaling_attachment
aws_autoscaling_group
aws_connect_hours_of_operation
aws_connect_queue
aws_connect_routing_profile
aws_elasticache_replication_group
aws_msk_cluster
aws_route_table

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

…rsion Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

ulucinar · 2024-02-01T17:36:18Z

/test-examples="examples/elasticache/v1beta1/replicationgroup-engineVersion.yaml"

sergenyalcin · 2024-02-01T17:36:33Z

/test-examples="examples/autoscaling/v1beta1/attachment.yaml"

sergenyalcin · 2024-02-01T17:39:02Z

/test-examples="examples/connect/v1beta1/routingprofile.yaml"

sergenyalcin · 2024-02-01T17:40:49Z

/test-examples="examples/ec2/v1beta1/route.yaml"

ulucinar

Thanks @sergenyalcin, lgtm.

sergenyalcin · 2024-02-01T18:36:15Z

The https://github.com/upbound/provider-aws/actions/runs/7745386352 job's import failed because of a known issue about the testing logic. The similar issue is here: #978 There is not any real issue in the resource lifecycle. This appears only import step of some resources.

mbbush · 2024-02-01T18:56:36Z

@sergenyalcin is this snippet from the uptest logs in https://github.com/upbound/provider-aws/actions/runs/7745386352 a concern?

2024-02-01T18:06:39Z	ERROR	conversion-webhook	failed to convert	{"request": "32e571f9-d6dc-4b47-ad9b-9a353750a57c", "error": "cannot convert from the hub version \"v1beta2\" to the spoke version \"v1beta1\": cannot apply the PavedConversion for the \"aws_route_table\" object: failed to get the field \"status.atProvider.route.networkInterfaceId\" from the conversion source object: status.atProvider.route: not an object", "errorVerbose": "status.atProvider.route: not an object\nfailed to get the field \"status.atProvider.route.networkInterfaceId\" from the conversion source object\ngithub.com/crossplane/upjet/pkg/config/conversion.(*fieldCopy).ConvertPaved\n\tgithub.com/crossplane/upjet@v1.1.0/pkg/config/conversion/conversions.go:102\ngithub.com/crossplane/upjet/pkg/controller/conversion.(*registry).RoundTrip\n\tgithub.com/crossplane/upjet@v1.1.0/pkg/controller/conversion/functions.go:40\ngithub.com/crossplane/upjet/pkg/controller/conversion.RoundTrip\n\tgithub.com/crossplane/upjet@v1.1.0/pkg/controller/conversion/functions.go:66\ngithub.com/upbound/provider-aws/apis/ec2/v1beta1.(*RouteTable).ConvertFrom\n\tgithub.com/upbound/provider-aws/apis/ec2/v1beta1/zz_generated.conversion_spokes.go:46\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).convertObject\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:140\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).handleConvertRequest\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:105\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).ServeHTTP\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:72\nsigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics.InstrumentedHook.InstrumentHandlerInFlight.func1\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:60\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:147\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:109\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\nnet/http.(*ServeMux).ServeHTTP\n\tnet/http/server.go:2514\nnet/http.serverHandler.ServeHTTP\n\tnet/http/server.go:2938\nnet/http.(*conn).serve\n\tnet/http/server.go:2009\nruntime.goexit\n\truntime/asm_amd64.s:1650\ncannot apply the PavedConversion for the \"aws_route_table\" object\ngithub.com/crossplane/upjet/pkg/controller/conversion.(*registry).RoundTrip\n\tgithub.com/crossplane/upjet@v1.1.0/pkg/controller/conversion/functions.go:41\ngithub.com/crossplane/upjet/pkg/controller/conversion.RoundTrip\n\tgithub.com/crossplane/upjet@v1.1.0/pkg/controller/conversion/functions.go:66\ngithub.com/upbound/provider-aws/apis/ec2/v1beta1.(*RouteTable).ConvertFrom\n\tgithub.com/upbound/provider-aws/apis/ec2/v1beta1/zz_generated.conversion_spokes.go:46\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).convertObject\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:140\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).handleConvertRequest\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:105\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).ServeHTTP\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:72\nsigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics.InstrumentedHook.InstrumentHandlerInFlight.func1\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:60\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:147\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:109\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\nnet/http.(*ServeMux).ServeHTTP\n\tnet/http/server.go:2514\nnet/http.serverHandler.ServeHTTP\n\tnet/http/server.go:2938\nnet/http.(*conn).serve\n\tnet/http/server.go:2009\nruntime.goexit\n\truntime/asm_amd64.s:1650\ncannot convert from the hub version \"v1beta2\" to the spoke version \"v1beta1\"\ngithub.com/upbound/provider-aws/apis/ec2/v1beta1.(*RouteTable).ConvertFrom\n\tgithub.com/upbound/provider-aws/apis/ec2/v1beta1/zz_generated.conversion_spokes.go:47\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).convertObject\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:140\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).handleConvertRequest\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:105\nsigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).ServeHTTP\n\tsigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:72\nsigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics.InstrumentedHook.InstrumentHandlerInFlight.func1\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:60\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:147\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2\n\tgithub.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:109\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2136\nnet/http.(*ServeMux).ServeHTTP\n\tnet/http/server.go:2514\nnet/http.serverHandler.ServeHTTP\n\tnet/http/server.go:2938\nnet/http.(*conn).serve\n\tnet/http/server.go:2009\nruntime.goexit\n\truntime/asm_amd64.s:1650"}
sigs.k8s.io/controller-runtime/pkg/webhook/conversion.(*webhook).ServeHTTP
	sigs.k8s.io/controller-runtime@v0.16.3/pkg/webhook/conversion/conversion.go:74
sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics.InstrumentedHook.InstrumentHandlerInFlight.func1
	github.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:60
net/http.HandlerFunc.ServeHTTP
	net/http/server.go:2136
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1
	github.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:147
net/http.HandlerFunc.ServeHTTP
	net/http/server.go:2136
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2
	github.com/prometheus/client_golang@v1.16.0/prometheus/promhttp/instrument_server.go:109
net/http.HandlerFunc.ServeHTTP
	net/http/server.go:2136
net/http.(*ServeMux).ServeHTTP
	net/http/server.go:2514
net/http.serverHandler.ServeHTTP
	net/http/server.go:2938
net/http.(*conn).serve
	net/http/server.go:2009

mbbush · 2024-02-01T18:57:40Z

Also, if we can get a new release of uptest, we'll be able to skip the import step per-resource with an annotation.

ulucinar · 2024-02-01T19:51:53Z

Hi @mbbush,
Thanks for taking a look at the logs. Looks like we have an issue in crossplane-runtime's fieldpath library. We will for now remove the affected converter and re-enable it after the issue is fixed upstream.

sergenyalcin force-pushed the multiversion-crds-conversions branch from cb1a28d to 14598be Compare January 31, 2024 12:05

sergenyalcin requested a review from erhancagirici January 31, 2024 14:28

sergenyalcin force-pushed the multiversion-crds-conversions branch 2 times, most recently from c8c5505 to 3d9a041 Compare January 31, 2024 23:21

sergenyalcin marked this pull request as ready for review January 31, 2024 23:29

sergenyalcin requested review from turkenf and ulucinar as code owners January 31, 2024 23:29

ulucinar mentioned this pull request Feb 1, 2024

Set CRD storage version to the latest generated version crossplane/upjet#339

Merged

3 tasks

sergenyalcin added 10 commits February 1, 2024 20:20

Add conversion functions

7798031

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Add unit tests and make target to checkout the apis to old release ve…

96fb090

…rsion Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Add storageVersions manually

05240e1

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Add override field configurations and make generate

721b313

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Return old types for the old apis

38a31a8

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Checkout ec2/zz_route_types.go to v0.47.1

be2e608

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Fix linter issues by setting //nolint:gocyclo comment to some functions

c58e01b

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Fix converters

044d98f

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Add example manifests for v1beta2 versions

4a42222

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

Fix reference issues and consume upjet v1.1.0

2afd235

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

sergenyalcin force-pushed the multiversion-crds-conversions branch from 0454a67 to 2afd235 Compare February 1, 2024 17:20

ulucinar approved these changes Feb 1, 2024

View reviewed changes

sergenyalcin merged commit 2dad70d into crossplane-contrib:main Feb 1, 2024

sergenyalcin deleted the multiversion-crds-conversions branch February 1, 2024 18:38

This was referenced Feb 1, 2024

Remove v1beta2 version of ec2.RouteTable #1124

Merged

Add a nil case to the getValueFromInterface function crossplane/crossplane-runtime#666

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add conversion functions for handling some breaking API changes#1118

Add conversion functions for handling some breaking API changes#1118
sergenyalcin merged 10 commits intocrossplane-contrib:mainfrom
sergenyalcin:multiversion-crds-conversions

sergenyalcin commented Jan 31, 2024 •

edited

Loading

Uh oh!

ulucinar commented Feb 1, 2024

Uh oh!

sergenyalcin commented Feb 1, 2024

Uh oh!

sergenyalcin commented Feb 1, 2024

Uh oh!

sergenyalcin commented Feb 1, 2024

Uh oh!

ulucinar left a comment

Uh oh!

sergenyalcin commented Feb 1, 2024

Uh oh!

mbbush commented Feb 1, 2024

Uh oh!

mbbush commented Feb 1, 2024

Uh oh!

ulucinar commented Feb 1, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sergenyalcin commented Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of your changes

How has this code been tested

Uh oh!

ulucinar commented Feb 1, 2024

Uh oh!

sergenyalcin commented Feb 1, 2024

Uh oh!

sergenyalcin commented Feb 1, 2024

Uh oh!

sergenyalcin commented Feb 1, 2024

Uh oh!

ulucinar left a comment

Choose a reason for hiding this comment

Uh oh!

sergenyalcin commented Feb 1, 2024

Uh oh!

mbbush commented Feb 1, 2024

Uh oh!

mbbush commented Feb 1, 2024

Uh oh!

ulucinar commented Feb 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sergenyalcin commented Jan 31, 2024 •

edited

Loading

ulucinar commented Feb 1, 2024 •

edited

Loading