From 4524830d33da8fb70e7e54271ee6a36c0bc2d977 Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Fri, 27 Nov 2020 13:10:17 +0000 Subject: [PATCH 1/3] Don't ignore errors when retrieving user_data_url --- ironic/resource_ironic_deployment.go | 17 ++++++++++------- ironic/resource_ironic_deployment_test.go | 5 ++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/ironic/resource_ironic_deployment.go b/ironic/resource_ironic_deployment.go index 4e0480b43..b75425a99 100644 --- a/ironic/resource_ironic_deployment.go +++ b/ironic/resource_ironic_deployment.go @@ -107,7 +107,10 @@ func resourceDeploymentCreate(d *schema.ResourceData, meta interface{}) error { userDataCaCert := d.Get("user_data_url_ca_cert").(string) // if user_data_url is specified in addition to user_data, use the former - ignitionData := fetchFullIgnition(userDataURL, userDataCaCert) + ignitionData, err := fetchFullIgnition(userDataURL, userDataCaCert) + if err != nil { + return fmt.Errorf("could not fetch data from user_data_url: %s", err) + } if ignitionData != "" { userData = ignitionData } @@ -125,7 +128,7 @@ func resourceDeploymentCreate(d *schema.ResourceData, meta interface{}) error { } // fetchFullIgnition gets full igntion from the URL and cert passed to it and returns userdata as a string -func fetchFullIgnition(userDataURL string, userDataCaCert string) string { +func fetchFullIgnition(userDataURL string, userDataCaCert string) (string, error) { // Send full ignition, if the URL is specified if userDataURL != "" { caCertPool := x509.NewCertPool() @@ -135,7 +138,7 @@ func fetchFullIgnition(userDataURL string, userDataCaCert string) string { caCert, err := base64.StdEncoding.DecodeString(userDataCaCert) if err != nil { log.Printf("could not decode user_data_url_ca_cert: %s", err) - return "" + return "", err } caCertPool.AppendCertsFromPEM(caCert) @@ -152,18 +155,18 @@ func fetchFullIgnition(userDataURL string, userDataCaCert string) string { resp, err := client.Get(userDataURL) if err != nil { log.Printf("could not get user_data_url: %s", err) - return "" + return "", err } defer resp.Body.Close() var userData []byte userData, err = ioutil.ReadAll(resp.Body) if err != nil { log.Printf("could not read user_data_url: %s", err) - return "" + return "", err } - return string(userData) + return string(userData), nil } - return "" + return "", nil } // buildConfigDrive handles building a config drive appropriate for the Ironic version we are using. Newer versions diff --git a/ironic/resource_ironic_deployment_test.go b/ironic/resource_ironic_deployment_test.go index 99926dcba..957a7da20 100644 --- a/ironic/resource_ironic_deployment_test.go +++ b/ironic/resource_ironic_deployment_test.go @@ -164,7 +164,10 @@ func TestFetchFullIgnition(t *testing.T) { }, } for _, tc := range testCases { - userData := fetchFullIgnition(tc.UserDataURL, tc.UserDataURLCACert) + userData, err := fetchFullIgnition(tc.UserDataURL, tc.UserDataURLCACert) + if err != nil { + t.Errorf("expected err: %s", err) + } if tc.ExpectResult && (userData != "Full Ignition\n") { t.Errorf("expected userData: %s, got %s", "Full Ignition\n", userData) } From 9a400fa970e4fbb51bb7ad55e0517af743a5f3b4 Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Fri, 27 Nov 2020 14:42:22 +0000 Subject: [PATCH 2/3] Add user_data_url_headers When retrieving user_data_url from some sources it's necessary to include headers to get the desired result, in particular when reading data from the MCS on OpenShift it's necessary to include "Accept: application/vnd.coreos.ignition+json; version=3.1.0" Otherwise the GET returns a 2.2.0 config which isn't compatible with the latest OS images. More info here: https://github.com/openshift/machine-config-operator/blob/master/docs/HACKING.md#accessing-the-machineconfigserver-directly --- ironic/resource_ironic_deployment.go | 22 +++++++++++++++++++--- ironic/resource_ironic_deployment_test.go | 3 ++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/ironic/resource_ironic_deployment.go b/ironic/resource_ironic_deployment.go index b75425a99..84f57e662 100644 --- a/ironic/resource_ironic_deployment.go +++ b/ironic/resource_ironic_deployment.go @@ -53,6 +53,11 @@ func resourceDeployment() *schema.Resource { Optional: true, ForceNew: true, }, + "user_data_url_headers": { + Type: schema.TypeMap, + Optional: true, + ForceNew: true, + }, "network_data": { Type: schema.TypeMap, Optional: true, @@ -105,9 +110,10 @@ func resourceDeploymentCreate(d *schema.ResourceData, meta interface{}) error { userData := d.Get("user_data").(string) userDataURL := d.Get("user_data_url").(string) userDataCaCert := d.Get("user_data_url_ca_cert").(string) + userDataHeaders := d.Get("user_data_url_headers").(map[string]interface{}) // if user_data_url is specified in addition to user_data, use the former - ignitionData, err := fetchFullIgnition(userDataURL, userDataCaCert) + ignitionData, err := fetchFullIgnition(userDataURL, userDataCaCert, userDataHeaders) if err != nil { return fmt.Errorf("could not fetch data from user_data_url: %s", err) } @@ -128,7 +134,7 @@ func resourceDeploymentCreate(d *schema.ResourceData, meta interface{}) error { } // fetchFullIgnition gets full igntion from the URL and cert passed to it and returns userdata as a string -func fetchFullIgnition(userDataURL string, userDataCaCert string) (string, error) { +func fetchFullIgnition(userDataURL string, userDataCaCert string, userDataHeaders map[string]interface{}) (string, error) { // Send full ignition, if the URL is specified if userDataURL != "" { caCertPool := x509.NewCertPool() @@ -152,7 +158,17 @@ func fetchFullIgnition(userDataURL string, userDataCaCert string) (string, error client.HTTPClient.Transport = transport // Get the data - resp, err := client.Get(userDataURL) + req, err := retryablehttp.NewRequest("GET", userDataURL, nil) + if err != nil { + log.Printf("could not get user_data_url: %s", err) + return "", err + } + if userDataHeaders != nil { + for k, v := range userDataHeaders { + req.Header.Add(k, v.(string)) + } + } + resp, err := client.Do(req) if err != nil { log.Printf("could not get user_data_url: %s", err) return "", err diff --git a/ironic/resource_ironic_deployment_test.go b/ironic/resource_ironic_deployment_test.go index 957a7da20..01cd0d51e 100644 --- a/ironic/resource_ironic_deployment_test.go +++ b/ironic/resource_ironic_deployment_test.go @@ -164,7 +164,8 @@ func TestFetchFullIgnition(t *testing.T) { }, } for _, tc := range testCases { - userData, err := fetchFullIgnition(tc.UserDataURL, tc.UserDataURLCACert) + emptyHeaders := make(map[string]interface{}) + userData, err := fetchFullIgnition(tc.UserDataURL, tc.UserDataURLCACert, emptyHeaders) if err != nil { t.Errorf("expected err: %s", err) } From e786855195bbd783130eb3c4bcf04ba8abdf4543 Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Mon, 30 Nov 2020 11:58:43 +0000 Subject: [PATCH 3/3] Add test for user_data_url_headers --- ironic/resource_ironic_deployment_test.go | 68 ++++++++++++++--------- 1 file changed, 41 insertions(+), 27 deletions(-) diff --git a/ironic/resource_ironic_deployment_test.go b/ironic/resource_ironic_deployment_test.go index 01cd0d51e..1793ee2a5 100644 --- a/ironic/resource_ironic_deployment_test.go +++ b/ironic/resource_ironic_deployment_test.go @@ -119,6 +119,11 @@ func testAccDeploymentResource(node, resourceClass, allocation string) string { func TestFetchFullIgnition(t *testing.T) { // Setup a fake https endpoint to server full ignition server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + for k, v := range r.Header { + if k == "Test" { + fmt.Fprintf(w, "Header: %s=%s\n", k, v) + } + } fmt.Fprintln(w, "Full Ignition") })) defer server.Close() @@ -131,49 +136,58 @@ func TestFetchFullIgnition(t *testing.T) { }, ) certB64 := base64.URLEncoding.EncodeToString(certInPem) + emptyHeaders := make(map[string]interface{}) testCases := []struct { - Scenario string - UserDataURL string - UserDataURLCACert string - ExpectResult bool + Scenario string + UserDataURL string + UserDataURLCACert string + UserDataURLHeaders map[string]interface{} + ExpectedResult string }{ { - Scenario: "user data url and ca cert present", - UserDataURL: server.URL, - UserDataURLCACert: certB64, - ExpectResult: true, + Scenario: "user data url and ca cert present", + UserDataURL: server.URL, + UserDataURLCACert: certB64, + UserDataURLHeaders: emptyHeaders, + ExpectedResult: "Full Ignition\n", }, { - Scenario: "user data url present but no ca cert", - UserDataURL: server.URL, - UserDataURLCACert: "", - ExpectResult: true, + Scenario: "user data url present but no ca cert", + UserDataURL: server.URL, + UserDataURLCACert: "", + UserDataURLHeaders: emptyHeaders, + ExpectedResult: "Full Ignition\n", }, { - Scenario: "user data url is not present but ca cert is", - UserDataURL: "", - UserDataURLCACert: certB64, - ExpectResult: false, + Scenario: "user data url, ca cert and headers present", + UserDataURL: server.URL, + UserDataURLCACert: certB64, + UserDataURLHeaders: map[string]interface{}{"Test": "foo"}, + ExpectedResult: "Header: Test=[foo]\nFull Ignition\n", }, { - Scenario: "neither user data url nor ca cert is not present", - UserDataURL: "", - UserDataURLCACert: "", - ExpectResult: false, + Scenario: "user data url is not present but ca cert is", + UserDataURL: "", + UserDataURLCACert: certB64, + UserDataURLHeaders: emptyHeaders, + ExpectedResult: "", + }, + { + Scenario: "neither user data url nor ca cert is not present", + UserDataURL: "", + UserDataURLCACert: "", + UserDataURLHeaders: emptyHeaders, + ExpectedResult: "", }, } for _, tc := range testCases { - emptyHeaders := make(map[string]interface{}) - userData, err := fetchFullIgnition(tc.UserDataURL, tc.UserDataURLCACert, emptyHeaders) + userData, err := fetchFullIgnition(tc.UserDataURL, tc.UserDataURLCACert, tc.UserDataURLHeaders) if err != nil { t.Errorf("expected err: %s", err) } - if tc.ExpectResult && (userData != "Full Ignition\n") { - t.Errorf("expected userData: %s, got %s", "Full Ignition\n", userData) - } - if !tc.ExpectResult && (userData != "") { - t.Errorf("expected userData: %s, got %s", "", userData) + if userData != tc.ExpectedResult { + t.Errorf("expected userData: %s, got %s", tc.ExpectedResult, userData) } } }