Implement policy site resource #1104
Conversation
ksamoray
force-pushed
the
resource_site
branch
2 times, most recently
from
a40097f to
4d2dbad
Compare
ksamoray
changed the title
ksamoray
force-pushed
the
resource_site
branch
2 times, most recently
from
23c7c76 to
fc76f7d
Compare
ksamoray
force-pushed
the
resource_site
branch
from
fc76f7d to
bfc60f2
Compare
| } | ||
|
|
||
| func resourceNsxtPolicySiteCreate(d *schema.ResourceData, m interface{}) error { | ||
| id, err := getOrGenerateID(d, m, resourceNsxtPolicySiteExists) |
There was a problem hiding this comment.
can we add here:
if !isPolicyGlobalManager(m) {
return globalManagerOnlyError()
}
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Description: "Persistent Site Type", | ||
| ValidateFunc: validation.StringInSlice(siteTypeValues, false), |
There was a problem hiding this comment.
No default here? What happens if this is unspecified? Should the value be Computed in this case?
There was a problem hiding this comment.
Spec doesn't specify a default here.
As UI in GM allows only on-prem addition, maybe we could discard this attribute entirely?
There was a problem hiding this comment.
I think this depends on env, and perhaps default depends on env as well. If NSX auto-assigns this value, we can define it as Computed.
There was a problem hiding this comment.
Turns out this this is required...
│ Error: Failed to create Site f5bb6846-cf9d-4efd-9cd6-4d11de593fff: Field level validation errors: {value of property site_type is not one of the allowed values [ONPREM_LM, SDDC_LM]} (code 255)
There was a problem hiding this comment.
Maybe no need to set a default for now.
Let's try and understand better the use cases around SDDC_LM. If ONPREM_LM is used in 99% of cases, then we can set if as default in a follow-up PR.
ksamoray
force-pushed
the
resource_site
branch
5 times, most recently
from
9611ac5 to
3c23ddb
Compare
| * `site_uuid` - (Optional) ID of Site. | ||
| * `thumbprint` - (Optional) Thumbprint of Enforcement Point. | ||
| * `username` - (Optional) Username. | ||
| * `site_type` - (Required) Persistent Site Type. Allowed values are ONPREM_LM, SDDC_LM. Default is ONPREM_LM. |
There was a problem hiding this comment.
It doesn't have a default according to the code (and its Required). Also lets surround the values in `` for consistency.
There was a problem hiding this comment.
It's required in the code? The spec doesn't enlist it as so.
ksamoray
force-pushed
the
resource_site
branch
from
3c23ddb to
7d399ee
Compare
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Description: "Persistent Site Type", | ||
| ValidateFunc: validation.StringInSlice(siteTypeValues, false), |
There was a problem hiding this comment.
Maybe no need to set a default for now.
Let's try and understand better the use cases around SDDC_LM. If ONPREM_LM is used in 99% of cases, then we can set if as default in a follow-up PR.
| State: nsxtPolicyPathResourceImporter, | ||
| }, | ||
|
|
||
| Schema: map[string]*schema.Schema{ |
There was a problem hiding this comment.
do you think site_number might be useful as a computed attribute?
Looking at the API spec, do you think we should include FederationConfig, remote_path, relative_path as computed too?
There was a problem hiding this comment.
Haven't found use for those yet, usually we add read-only attrs only if they're used as reference anywhere else or just seem very useful. As I haven't complete yet the federation object implementation, I might find their use later on maybe. Anyway it's always easier to add attributes than removing them.
nsxt/resource_nsxt_policy_site.go
Outdated
| var tags []gm_model.Tag | ||
| tagListIntface := d.Get("tag") | ||
| if tagListIntface != nil { | ||
| tagList := tagListIntface.(*schema.Set).List() |
There was a problem hiding this comment.
I might be wrong, but didn't we have util method for doing this process?
There was a problem hiding this comment.
We have more than one utility, as the SDK has different model.Tag structs for policy/MP/GM APIs. However there's no util func for GM - I could add one or convert to policy model and then use TypeConverter (we do this when there's a resource which runs for both M and policy).
| connector := getPolicyConnector(m) | ||
| client := global_infra.NewSitesClient(connector) | ||
|
|
||
| obj := getSiteFromSchema(d) |
There was a problem hiding this comment.
It's not clear to me the behaviour that NSX will have if the update site_connection_info.
Did we test it?
There was a problem hiding this comment.
I didn't try to set it (I need an extra LM to test this). But connectivity info is updatable in UI.
| * `nsx_id` - (Optional) The NSX ID of this resource. If set, this ID will be used to create the resource. | ||
| * `fail_if_rtep_misconfigured` - (Optional) Fail onboarding if RTEPs misconfigured. Default is true. | ||
| * `fail_if_rtt_exceeded` - (Optional) Fail onboarding if maximum RTT exceeded. Default is true. | ||
| * `maximum_rtt` - (Optional) Maximum acceptable packet round trip time (RTT). Default is 250. |
There was a problem hiding this comment.
The NSX API guide does not mention it, but I think we should specify the unit of measure.
msecs, I believe.
There was a problem hiding this comment.
IDK which are the units as I couldn't find that attribute in the UI
ksamoray
force-pushed
the
resource_site
branch
11 times, most recently
from
1355ddc to
6d4fd70
Compare
Signed-off-by: Kobi Samoray <[email protected]>
No description provided.