Skip to content
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

This commit contains entire DNS changes #25

Merged
merged 13 commits into from
Mar 22, 2019
Merged

This commit contains entire DNS changes #25

merged 13 commits into from
Mar 22, 2019

Conversation

saiprasannasastry
Copy link
Contributor

This commit contains support for A,CNAME,PTR and Host records

UT Results
PASS
ok github.com/infobloxopen/terraform-provider-infoblox/infoblox 203.885s

Acceptance UT Results
=== RUN TestProvider
--- PASS: TestProvider (0.00s)
=== RUN TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN TestValidationStringInSlice
--- PASS: TestValidationStringInSlice (0.00s)
=== RUN TestAccResourceARecord
--- PASS: TestAccResourceARecord (29.22s)
=== RUN TestAccResourceCNAMERecord
--- PASS: TestAccResourceCNAMERecord (29.74s)
=== RUN TestAccResourceIPAllocation
--- PASS: TestAccResourceIPAllocation (31.28s)
=== RUN TestAccresourceIPAssociation
--- PASS: TestAccresourceIPAssociation (27.37s)
=== RUN TestAccresourceNetworkView
--- PASS: TestAccresourceNetworkView (16.46s)
=== RUN TestAccresourceNetwork
--- PASS: TestAccresourceNetwork (40.75s)
=== RUN TestAccResourcePTRRecord
--- PASS: TestAccResourcePTRRecord (29.06s)

This commit contains support for A,CNAME,PTR and Host records

UT Results
PASS
ok      github.com/infobloxopen/terraform-provider-infoblox/infoblox    203.885s

Acceptance UT Results
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestValidationStringInSlice
--- PASS: TestValidationStringInSlice (0.00s)
=== RUN   TestAccResourceARecord
--- PASS: TestAccResourceARecord (29.22s)
=== RUN   TestAccResourceCNAMERecord
--- PASS: TestAccResourceCNAMERecord (29.74s)
=== RUN   TestAccResourceIPAllocation
--- PASS: TestAccResourceIPAllocation (31.28s)
=== RUN   TestAccresourceIPAssociation
--- PASS: TestAccresourceIPAssociation (27.37s)
=== RUN   TestAccresourceNetworkView
--- PASS: TestAccresourceNetworkView (16.46s)
=== RUN   TestAccresourceNetwork
--- PASS: TestAccresourceNetwork (40.75s)
=== RUN   TestAccResourcePTRRecord
--- PASS: TestAccResourcePTRRecord (29.06s)
@sganesh-infoblox
Copy link

From Cloud perspective changes are fine.

Copy link

@sganesh-infoblox sganesh-infoblox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks fine from Cloud perspective.

@sganesh-infoblox
Copy link

@saiprasannasastry Please add @AjeyHiremath & @jkraj as reviewers.


#Using the templates for different combinations.

- For cretion of fixedAddress or if you just using the provider without dns,
Copy link
Collaborator

@jkraj jkraj Mar 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. typo - creation
  2. Make keyword bold and check the case
  3. Give explict names

@@ -34,11 +34,11 @@ func Provider() terraform.ResourceProvider {
"wapi_version": &schema.Schema{
Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("WAPI_VERSION", "2.8"),
Description: "WAPI Version of Infoblox server defaults to v2.8.",
DefaultFunc: schema.EnvDefaultFunc("WAPI_VERSION", "2.10"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep some stable lower version

Type: schema.TypeString,
Required: true,
DefaultFunc: schema.EnvDefaultFunc("net_address", nil),
Description: "Give the address in cidr format.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove 'Give the'

Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("dns_view", nil),
Description: "Dns View under which the zone has been created .",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space before dot

connector := m.(*ibclient.Connector)

objMgr := ibclient.NewObjectManager(connector, "terraform", tenantID)
name := Name + "." + zone
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change name => fqdn


networkViewName := d.Get("network_view_name").(string)
//This is for record Name
Name := d.Get("vm_name").(string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name => recordName

},
"dns_view": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required = true, check in all A, PTR as well

Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("zone", nil),
Description: "Zone under which record has to be created .",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after dot, check in other places as well

Schema: map[string]*schema.Schema{
"zone": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Copy link
Collaborator

@jkraj jkraj Mar 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required == true

Type: schema.TypeBool,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("enable_dns", nil),
Description: "flag that defines if the host reocrd is used for DNS or IPAM Puurposes .",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: puurposes

"ip_addr": &schema.Schema{
Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("ipaddr", nil),
Description: "IP address of your instance in cloud.",
Description: "IP address you want to allocate yourinstance in cloud.If field is not specified , it akes next avaliable ip address",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

punctuation,
IP address of your instance in cloud. For static set ip_addr for dynamic leave this empty.

func resourceIPAllocationRequest(d *schema.ResourceData, m interface{}) error {
log.Printf("[DEBUG] %s: Beginning to request a next free IP from a required network block", resourceIPAllocationIDString(d))

networkViewName := d.Get("network_view_name").(string)
recordName := d.Get("host_name").(string)
//This is for record Name
Name := d.Get("vm_name").(string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change this to meaninful variable names

//a host record which doesnt have the details of the mac address
//at the beginigand we are using this update call to update the mac address
//a reservation which doesnt have the details of the mac address
//at the beginig and we are using this update call to update the mac address
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo begining

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beginig ==> beginning

if err != nil {
return fmt.Errorf("Error Releasing IP from network block having reference (%s): %s", d.Id(), err)
if (zone != "" || len(zone) != 0) && (dnsView != "" || len(dnsView) != 0) {
_, err := objMgr.UpdateHostRecord(d.Id(), ipAddr, "00:00:00:00:00:00", vmID, vmName)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use variable for zero mac

Copy link
Collaborator

@jkraj jkraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments

@saiprasannasastry
Copy link
Contributor Author

@jkraj please review

@@ -73,7 +73,7 @@ func resourceARecordCreate(d *schema.ResourceData, m interface{}) error {

networkViewName := d.Get("network_view_name").(string)
//This is for record Name
Name := d.Get("vm_name").(string)
record_Name := d.Get("vm_name").(string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In go u don't have to use '_' so record_Name ==> recordName

func resourceCNAMERecordCreate(d *schema.ResourceData, m interface{}) error {
log.Printf("[DEBUG] %s: Beginning to create CNAME record ", resourceCNAMERecordIDString(d))

Zone := d.Get("zone").(string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

z not Z


Zone := d.Get("zone").(string)
dnsView := d.Get("dns_view").(string)
Canonical := d.Get("canonical").(string) + "." + Zone
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c not C

},
"ip_addr": &schema.Schema{
Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("ipaddr", nil),
Description: "IP address of your instance in cloud.",
Description: "IP address your instance in cloud.For static allocation ,set the field. For dynamic allocation, leave this field empty.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set the field => set this field with valid IP.

hostrecordObj, err := objMgr.CreateHostRecordWithoutDNS(recordName, networkViewName, cidr, ipAddr, macAddr, vmID)
if err != nil {
return fmt.Errorf("Error allocating IP from network block(%s): %s", cidr, err)
if (zone != "" || len(zone) != 0) && (dnsView != "" || len(dnsView) != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both check are same right. why we need this 2 check.

zone != "" == len(zone) != 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zone != "" == len(zone) != 0 cant be done because
invalid operation: dnsView != "" == len(dnsView) (mismatched types bool and int)
i've see the core NIOS code have these kind of check that is why i added them

@@ -95,21 +107,32 @@ func resourceIPAssociationRead(d *schema.ResourceData, m interface{}) error {
//will be a conflict of resources
func resourceIPAssociationDelete(d *schema.ResourceData, m interface{}) error {
log.Printf("[DEBUG] %s: Beginning Reassociation of IP address in specified network block", resourceIPAssociationIDString(d))

match_client := "MAC_ADDRESS"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

matchClient


connector := m.(*ibclient.Connector)

ZERO_MACADDR := "00:00:00:00:00:00"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZERO_MACADDR ==> zeroMacAddr

_, err := objMgr.UpdateHostRecordWithoutDNS(d.Id(), ipAddr, "00:00:00:00:00:00", "")
if err != nil {
return fmt.Errorf("Error Releasing IP from network block having reference (%s): %s", d.Id(), err)
if (zone != "" || len(zone) != 0) && (dnsView != "" || len(dnsView) != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same us above.

networkViewName := d.Get("network_view_name").(string)
recordName := d.Get("host_name").(string)
Name := d.Get("vm_name").(string)
Copy link
Collaborator

@jkraj jkraj Mar 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not just 'Name' => vmName

_, err = objMgr.UpdateHostRecordWithoutDNS(hostRecordObj.Ref, ipAddr, macAddr, vmID)
if err != nil {
return fmt.Errorf("UpdateHostAddress error from network block(%s):%s", cidr, err)
if (zone != "" || len(zone) != 0) && (dnsView != "" || len(dnsView) != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Collaborator

@jkraj jkraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comments,

mainly on variable name
one if check case.

@jkraj jkraj merged commit 71f6216 into infobloxopen:master Mar 22, 2019
@saiprasannasastry saiprasannasastry deleted the dnschanges branch September 3, 2019 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants