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.

Sign up for GitHub

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

Add IAP support for backend services #471

Merged
merged 9 commits into from
Oct 31, 2017
Merged

Add IAP support for backend services #471

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ae5ad95
initial work on adding IAP support for backend services
Sep 27, 2017
79df895
readback of IAP
Sep 27, 2017
7b02cdb
flatten IAP + static set id
Sep 27, 2017
b67c08e
expandIap function
Sep 27, 2017
e865ea2
removed enabled flag/state rework
Sep 28, 2017
285b10b
Tests for IAP
Sep 29, 2017
9de4d86
added comments, fixed tabs.
Oct 21, 2017
ec4afb2
merge master
Oct 21, 2017
95bb1c6
testing for IAP disabled
Oct 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion google/resource_compute_backend_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,32 @@ func resourceComputeBackendService() *schema.Resource {
MaxItems: 1,
},

"iap": &schema.Schema{
Type: schema.TypeSet,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's MaxItems: 1 anyway, I'd recommend just making this a list. Then you can also get rid of the Set fn on line 63

Elem: &schema.Resource{
Copy link
Contributor

Choose a reason for hiding this comment

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

Most of our other resources have Elem as the last item in the struct. I'm not sure why this resource does it the other way but it would be awesome if you could move this one and change some of the others in this file too!

Schema: map[string]*schema.Schema{
"enabled": &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether we should make this Required, that way people don't accidentally try to enable it by putting in the block but forgetting this field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered this as well. I think about how I work, and commenting out IAP enabled would "disable" it, without having to edit the line and set it as false. What's the standard here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something else to consider here, should we even have the enabled flag? Should it be implicit by providing the keys?

Default: false,
},
"oauth2_client_id": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Default: "",
},
"oauth2_client_secret": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Default: "",
},
},
},
Optional: true,
MaxItems: 1,
Set: func(i interface{}) int { return 0 },
},

"backend": &schema.Schema{
Type: schema.TypeSet,
Elem: &schema.Resource{
Expand Down Expand Up @@ -203,6 +229,7 @@ func resourceComputeBackendServiceRead(d *schema.ResourceData, meta interface{})
d.Set("self_link", service.SelfLink)
d.Set("backend", flattenBackends(service.Backends))
d.Set("connection_draining_timeout_sec", service.ConnectionDraining.DrainingTimeoutSec)
d.Set("iap", flattenIap(service.Iap))
Copy link
Contributor

Choose a reason for hiding this comment

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

will service.Iap ever be nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only in-so-far as the service object doesn't initialize it a few lines up from here. I'll add a check in the flatten function.


d.Set("health_checks", service.HealthChecks)

Expand Down Expand Up @@ -261,6 +288,27 @@ func resourceComputeBackendServiceDelete(d *schema.ResourceData, meta interface{
return nil
}

func expandIap(configured []interface{}) *compute.BackendServiceIAP {
data := configured[0].(map[string]interface{})
iap := &compute.BackendServiceIAP{
Enabled: data["enabled"].(bool),
Copy link
Contributor

Choose a reason for hiding this comment

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

Without also setting ForceSendFields, these three fields won't appear in the request if they're set to empty (or false). It might still work since the outer block will still be there, but if you could check that you can disable iap using this code that would be great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will double check and also add tests.

Oauth2ClientId: data["oauth2_client_id"].(string),
Oauth2ClientSecret: data["oauth2_client_secret"].(string),
}
return iap
}

func flattenIap(iap *compute.BackendServiceIAP) []map[string]interface{} {
result := make([]map[string]interface{}, 0, 1)
iapMap := map[string]interface{}{
"enabled": iap.Enabled,
"oauth2_client_id": iap.Oauth2ClientId,
"oauth2_client_secret": iap.Oauth2ClientSecret,
}
result = append(result, iapMap)
Copy link
Contributor

Choose a reason for hiding this comment

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

Totally up to you, but a shorter way to do this would be:

iapMap := map[string]interface{}{
  // properties
}
return []map[string]interface{}{iapMap}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't considered that -- I agree, much cleaner.

return result
}

func expandBackends(configured []interface{}) []*compute.Backend {
backends := make([]*compute.Backend, 0, len(configured))

Expand Down Expand Up @@ -309,7 +357,6 @@ func flattenBackends(backends []*compute.Backend) []map[string]interface{} {
data["max_rate"] = b.MaxRate
data["max_rate_per_instance"] = b.MaxRatePerInstance
data["max_utilization"] = b.MaxUtilization

result = append(result, data)
}

Expand All @@ -328,6 +375,10 @@ func expandBackendService(d *schema.ResourceData) compute.BackendService {
HealthChecks: healthChecks,
}

if v, ok := d.GetOk("iap"); ok {
service.Iap = expandIap(v.(*schema.Set).List())
}

if v, ok := d.GetOk("backend"); ok {
service.Backends = expandBackends(v.(*schema.Set).List())
}
Expand Down