Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Move to CLI schema #169

Merged
merged 1 commit into from
Apr 4, 2022
Merged

Conversation

ezgidemirel
Copy link
Collaborator

@ezgidemirel ezgidemirel commented Mar 11, 2022

Description of your changes

This change updates the provider to use CLI schema. This change is on top of this PR.

Fixes #166

I have:

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

How has this code been tested

Created/deleted KMS, Neptune Cluster and Neptune Instance resources.

NAME                                     READY   SYNCED   EXTERNAL-NAME                          AGE
key.kms.aws.jet.crossplane.io/ezgi-key   True    True     46964507-81f5-4b23-9bc2-15dc87a2c1b1   15m

NAME                                                          READY   SYNCED   EXTERNAL-NAME   AGE
clusterinstance.neptune.aws.jet.crossplane.io/ezgi-instance   True    True     ezgi-instance   10m

NAME                                                 READY   SYNCED   EXTERNAL-NAME   AGE
cluster.neptune.aws.jet.crossplane.io/ezgi-cluster   True    True     ezgi-cluster    13m

Copy link
Member

@muvaf muvaf left a comment

Choose a reason for hiding this comment

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

Thanks @ezgidemirel !

@@ -183,12 +176,7 @@ spec:
self:
type: boolean
toPort:
format: int64
type: integer
required:
Copy link
Member

Choose a reason for hiding this comment

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

Why are these not required anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They're not marked as "required": true in the cli schema. I generated preview CRDs and noticed the same change in some of the other resources too.

Copy link
Member

Choose a reason for hiding this comment

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

Could you note this case here or open an issue so that we can investigate it? Ideally, we shouldn't lose any information due to conversions done by TF CLI or us. Noting it down somewhere would help us know what to fix without trying again.

go.mod Show resolved Hide resolved
@muvaf muvaf mentioned this pull request Mar 14, 2022
2 tasks
Signed-off-by: ezgidemirel <ezgidemirel91@gmail.com>
@muvaf muvaf merged commit b360fee into crossplane-contrib:main Apr 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to use CLI schema
2 participants