Move to CLI schema #161
Move to CLI schema #161
Conversation
ezgidemirel
force-pushed
the
move-to-cli-schema
branch
from
9fc37bf to
dafb65f
Compare
ezgidemirel
force-pushed
the
move-to-cli-schema
branch
6 times, most recently
from
883081d to
43af7d3
Compare
|
Hi @ezgidemirel, |
ezgidemirel
force-pushed
the
move-to-cli-schema
branch
from
43af7d3 to
221675a
Compare
rebased :) |
ezgidemirel
force-pushed
the
move-to-cli-schema
branch
from
221675a to
4222aff
Compare
ezgidemirel
force-pushed
the
move-to-cli-schema
branch
from
4222aff to
4c64a43
Compare
There was a problem hiding this comment.
Thank you @ezgidemirel for working on this, very much appreciated. I've left a few nits but the PR looks good to me. Could you please point me to an MR API change which introduces floats? Could you please also rebase the PR?
| @@ -30,6 +31,12 @@ import ( | |||
| func Configure(p *config.Provider) { | |||
| p.AddResourceConfigurator("azurerm_iothub", func(r *config.Resource) { | |||
| r.Version = common.VersionV1Alpha2 | |||
| r.TerraformResource.Schema["endpoint"].Elem.(*schema.Resource). | |||
There was a problem hiding this comment.
nit: We may consider documenting why we need to explicitly mark these as sensitive with code comments. If we have a related issue, we may also refer to that issue.
| @@ -31,6 +32,22 @@ import ( | |||
| func Configure(p *config.Provider) { | |||
| p.AddResourceConfigurator("azurerm_kubernetes_cluster", func(r *config.Resource) { | |||
| r.Version = common.VersionV1Alpha2 | |||
| r.TerraformResource.Schema["kube_admin_config"].Elem.(*schema.Resource). | |||
There was a problem hiding this comment.
nit: We may consider documenting why we need to explicitly mark these as sensitive with code comments. If we have a related issue, we may also refer to that issue.
| @@ -49,10 +50,8 @@ import ( | |||
| "github.com/crossplane-contrib/provider-jet-azure/config/subnet" | |||
| ) | |||
|
|
|||
| const ( | |||
| resourcePrefix = "azure" | |||
| modulePath = "github.com/crossplane-contrib/provider-jet-azure" | |||
There was a problem hiding this comment.
Looks like we still need this constant in line 136.
There was a problem hiding this comment.
added it back :)
config/provider.go
Outdated
| @@ -49,10 +50,8 @@ import ( | |||
| "github.com/crossplane-contrib/provider-jet-azure/config/subnet" | |||
| ) | |||
|
|
|||
| const ( | |||
| resourcePrefix = "azure" | |||
There was a problem hiding this comment.
Looks like we still need this constant in line 136.
There was a problem hiding this comment.
added it back :)
Signed-off-by: ezgidemirel <[email protected]>
ezgidemirel
force-pushed
the
move-to-cli-schema
branch
from
4c64a43 to
49d0cde
Compare
|
@ezgidemirel, could you please also take a look at the upgrade procedure? I assume we will need to do a minor version bump and because of the breaking API changes, we will not have a smooth upgrade, correct? |
I've tested the upgrade scenario by updating v0.8.0 provider package to the one built on my branch and there were no issues. |
There was a problem hiding this comment.
Thank you very much @ezgidemirel for working on this! Very much appreciated.
Description of your changes
This change updates the provider to use CLI schema. It is opened on top of this PR.
Fixes #142
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested