-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
prepare provider for tfplugindocs #83
Conversation
- `ca_certificate` (String) Path to MikroTik's certificate authority | ||
- `host` (String) Hostname of the MikroTik router | ||
- `insecure` (Boolean) Insecure connection does not verify MikroTik's TLS certificate | ||
- `password` (String) Password for MikroTik api | ||
- `tls` (Boolean) Whether to use TLS when connecting to MikroTik or not | ||
- `username` (String) User account for MikroTik api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
username, password and host are not optional fields. So it seems like the provider's arguments aren't set correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's how it is declared:
"username": {
Type: schema.TypeString,
Required: true,
DefaultFunc: schema.EnvDefaultFunc("MIKROTIK_USER", nil),
Description: "User account for mikrotik api",
},
so yes, it should either be set to Optional
or update the docs and mention that there is a fallback to env vars.
I'd like to merge this PR first so we can rebase the others to remove the shared changes. I had one question about why everything is considered optional, otherwise this looks good. |
@ddelnano we can turn those fields into |
I see. I didn't realize that having the env in use would make these optional. Anyway, knowing that this is good as is. |
@Maxim-Nazarenko please rebase the remaining doc update PRs and then we can get those merged. |
@ddelnano I just relaised what you was referring to. Sharp eye! |
Part of #82