ddelnano / terraform-provider-mikrotik

MIT License
128 stars 28 forks source link

Scheduler does not support duration notations #93

Closed maksym-nazarenko closed 1 year ago

maksym-nazarenko commented 2 years ago

How to reproduce:

  1. Create a scheduler on MikroTik:
    
    /system script add name="configBackup" owner="user" policy=read,write,policy,test,password,sensitive dont-require-permissions=no source=":put done"

/system scheduler add name="AutomatedBackup" start-date=aug/28/2018 start-time=03:15:00 interval=1w on-event=configBackup owner="user" policy=read,write,policy,test,password,sensitive


2. Define a resource:
```terraform
resource "mikrotik_scheduler" "backup" {
  name     = "AutomatedBackup"
  interval = 1 * 7 * 86400 # 1 week
  on_event = mikrotik_script.backup.name
}
  1. Try importing the resource:
terraform import mikrotik_scheduler.backup AutomatedBackup

Expected behavior The resource is imported.

Actual behavior Provider panics

Show code ```sh ╷ │ Error: Request cancelled │ │ The plugin.(*GRPCProvider).ReadResource request was cancelled. ╵ Stack trace from the terraform-provider-mikrotik_v0.9.1 plugin: panic: time: unknown unit "w" in duration "1w" goroutine 57 [running]: github.com/ddelnano/terraform-provider-mikrotik/client.ttlToSeconds({0xc0000a1088, 0x2}) github.com/ddelnano/terraform-provider-mikrotik/client@v0.0.0-00010101000000-000000000000/client.go:132 +0x137 github.com/ddelnano/terraform-provider-mikrotik/client.parseStruct(0x161a800, {{0xc0000a0f67, 0x3}, {0x0, 0x0}, {0xc000670000, 0xc, 0x10}, 0xc00062fad0}) github.com/ddelnano/terraform-provider-mikrotik/client@v0.0.0-00010101000000-000000000000/client.go:93 +0x46d github.com/ddelnano/terraform-provider-mikrotik/client.Unmarshal({{0xc0000aed28, 0xc0000c8f80, 0xc000197808}, 0xc00004d780}, {0x161a800, 0xc0004810e0}) github.com/ddelnano/terraform-provider-mikrotik/client@v0.0.0-00010101000000-000000000000/client.go:68 +0x308 github.com/ddelnano/terraform-provider-mikrotik/client.Mikrotik.FindScheduler({{0xc0000a0270, 0xb}, {0xc0000a02a0, 0x9}, {0xc00003e112, 0x26}, 0x1, {0x0, 0x0}, 0x1, ...}, ...) github.com/ddelnano/terraform-provider-mikrotik/client@v0.0.0-00010101000000-000000000000/scheduler.go:30 +0x17c github.com/ddelnano/terraform-provider-mikrotik/mikrotik.resourceSchedulerRead({0x1812b98, 0xc00004d100}, 0xc0000fce00, {0x17186a0, 0xc0004804e0}) github.com/ddelnano/terraform-provider-mikrotik/mikrotik/resource_scheduler.go:63 +0x167 github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0xc000164380, {0x1812b98, 0xc00004d100}, 0x24, {0x17186a0, 0xc0004804e0}) github.com/hashicorp/terraform-plugin-sdk/v2@v2.9.0/helper/schema/resource.go:358 +0x12e github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).RefreshWithoutUpgrade(0xc000164380, {0x1812b98, 0xc00004d100}, 0xc0004bea90, {0x17186a0, 0xc0004804e0}) github.com/hashicorp/terraform-plugin-sdk/v2@v2.9.0/helper/schema/resource.go:635 +0x35b github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadResource(0xc00000c090, {0x1812b98, 0xc00004d100}, 0xc00004d140) github.com/hashicorp/terraform-plugin-sdk/v2@v2.9.0/helper/schema/grpc_provider.go:576 +0x534 github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0xc00007cc80, {0x1812c40, 0xc00062ed80}, 0xc000480660) github.com/hashicorp/terraform-plugin-go@v0.4.0/tfprotov5/tf5server/server.go:298 +0x1fb github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadResource_Handler({0x16f9780, 0xc00007cc80}, {0x1812c40, 0xc00062ed80}, 0xc000480600, 0x0) github.com/hashicorp/terraform-plugin-go@v0.4.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:344 +0x170 google.golang.org/grpc.(*Server).processUnaryRPC(0xc000124700, {0x181e7f8, 0xc00051e000}, 0xc000158200, 0xc00039a5d0, 0x1c90c90, 0x0) google.golang.org/grpc@v1.32.0/server.go:1194 +0xc8f google.golang.org/grpc.(*Server).handleStream(0xc000124700, {0x181e7f8, 0xc00051e000}, 0xc000158200, 0x0) google.golang.org/grpc@v1.32.0/server.go:1517 +0xa2a google.golang.org/grpc.(*Server).serveStreams.func1.2() google.golang.org/grpc@v1.32.0/server.go:859 +0x98 created by google.golang.org/grpc.(*Server).serveStreams.func1 google.golang.org/grpc@v1.32.0/server.go:857 +0x294 Error: The terraform-provider-mikrotik_v0.9.1 plugin crashed! This is always indicative of a bug within the plugin. It would be immensely helpful if you could report the crash with the plugin's maintainers so that it can be fixed. The output above should help diagnose the issue. ```

The issue here is:

panic: time: unknown unit "w" in duration "1w" The time.Duration does not support w as suffix and client.ttlToSeconds() supprts either d suffix or falls back to time.ParseDuration().

Suggestion We could leverage json package approach with equivalent of MarshalJSON() in interface.

Something like this:

type Marshaler interface {
    MarshalMikrotik() (string, error)
}

type Unmarshaler interface {
    UnmarshalMikrotik(string) error
}

And then implement custom MikrotikDuration type that satisfies interfaces and can marshal/unmarshal MikroTik's values.

Another thing addressed by this approach would be proper handling of fields like:

type Script {
    Policies StringSliceField
}

Where StringSliceField can be marshalled/unmarshalled in a way: []string{"read", "write", "romon"} <-> read,write,romon

maksym-nazarenko commented 1 year ago

Since #119 is already merged, we can implement custom duration type to handle all edge cases properly