Skip to content

Commit

Permalink
review: make path property required
Browse files Browse the repository at this point in the history
for source_file with storage

Signed-off-by: Lucas Bremgartner <[email protected]>
  • Loading branch information
breml committed Nov 26, 2024
1 parent 6f6050e commit df320a9
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 11 deletions.
3 changes: 2 additions & 1 deletion docs/resources/instance.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ resource "incus_instance" "instance1" {
In order to provide the storage pool name for an instance, which is created
from a backup exactly one `device` configuration of `type = "disk"` might be
provided. The name of the pool is given as the `pool` attribute in
`properties`, which is the only property accepted in this case.
`properties`. Additionally the property `path = "/"` is required.

```hcl
resource "incus_instance" "instance1" {
Expand All @@ -116,6 +116,7 @@ resource "incus_instance" "instance1" {
name = "storage"
type = "disk"
properties = {
path = "/"
pool = "pool-name"
}
}
Expand Down
23 changes: 13 additions & 10 deletions internal/instance/resource_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,8 @@ func (r InstanceResource) ValidateConfig(ctx context.Context, req resource.Valid

// With `incus import`, a storage pool can be provided optionally.
// In order to support the same behavior with source_file,
// a single device entry of type `disk` is allowed with a single property
// `pool` set to the respective pool name.
// a single device entry of type `disk` is allowed with exactly two properties
// `path` and `pool` being set. For `path`, the only accepted value is `/`.
if len(config.Devices.Elements()) > 0 {
if len(config.Devices.Elements()) > 1 {
resp.Diagnostics.AddError(
Expand All @@ -427,25 +427,28 @@ func (r InstanceResource) ValidateConfig(ctx context.Context, req resource.Valid
return
}

if len(deviceList[0].Properties.Elements()) != 1 {
if len(deviceList[0].Properties.Elements()) != 2 {
resp.Diagnostics.AddError(
"Invalid Configuration",
`Exactly one device property named "pool" needs to be provided with source_file.`,
`Exactly two device properties named "path" and "pool" need to be provided with source_file.`,
)
return
}

properties := make(map[string]string, 1)
properties := make(map[string]string, 2)
diags = deviceList[0].Properties.ElementsAs(ctx, &properties, false)
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}

if _, ok := properties["pool"]; !ok {
_, poolOK := properties["pool"]
path, pathOK := properties["path"]

if !poolOK || !pathOK || path != "/" {
resp.Diagnostics.AddError(
"Invalid Configuration",
`Exactly one device property named "pool" needs to be provided with source_file.`,
`Exactly two device properties named "path" and "pool" need to be provided with source_file. For "path", the only accepted value is "/".`,
)
return
}
Expand Down Expand Up @@ -872,7 +875,7 @@ func (r InstanceResource) SyncState(ctx context.Context, tfState *tfsdk.State, s
}

if !m.SourceFile.IsNull() && !m.Devices.IsNull() {
// Using device to signal the storage poll is a special case, which is not
// Using device to signal the storage pool is a special case, which is not
// reflected on the instance state and therefore we need to compensate here
// in order to prevent inconsistent provider results.
devices = m.Devices
Expand Down Expand Up @@ -987,8 +990,8 @@ func (r InstanceResource) createInstanceFromSourceFile(ctx context.Context, serv
return diags
}

// Exactly one property named "pool" is expected, this is ensured by ValidateConfig.
properties := make(map[string]string, 1)
// Exactly two properties named "path" and "pool" are expected, this is ensured by ValidateConfig.
properties := make(map[string]string, 2)
diags = deviceList[0].Properties.ElementsAs(ctx, &properties, false)
if diags.HasError() {
return diags
Expand Down
2 changes: 2 additions & 0 deletions internal/instance/resource_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,7 @@ func TestAccInstance_sourceFileWithStorage(t *testing.T) {
resource.TestCheckResourceAttr("incus_instance.instance1", "device.#", "1"),
resource.TestCheckResourceAttr("incus_instance.instance1", "device.0.name", "storage"),
resource.TestCheckResourceAttr("incus_instance.instance1", "device.0.type", "disk"),
resource.TestCheckResourceAttr("incus_instance.instance1", "device.0.properties.path", "/"),
resource.TestCheckResourceAttr("incus_instance.instance1", "device.0.properties.pool", "default"),
),
},
Expand Down Expand Up @@ -1603,6 +1604,7 @@ resource "incus_instance" "instance1" {
name = "storage"
type = "disk"
properties = {
"path" = "/"
"pool" = "default"
}
}
Expand Down

0 comments on commit df320a9

Please sign in to comment.