Repository: metal3-io/baremetal-operator
Pull request #924 (2021-06-28 08:16:35)

Add TLS check before generating the secret for ironic-cacert secret.

metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **russellb** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @russellb` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["russellb"]} -->
smoshiur1237: /test-integration
metal3-io-bot: @smoshiur1237: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
shellcheck | 0f46775e5141f20e0b574abfaa2543041df328b4 | [link](https://prow.apps.test.metal3.io/view/gs/metal3-prow/pr-logs/pull/metal3-io_baremetal-operator/924/shellcheck/1410528890006278144) | `/test shellcheck`

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/docs/api.md b/docs/api.md
index d18e2c4f..a5c8bb4e 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -32,9 +32,6 @@ communicate with them.
 * IPMI
   * `ipmi://<host>:<port>`, an unadorned `<host>:<port>` is also accepted
     and the port is optional, if using the default one (623).
-  * The ipmi privilege level can be set from the default(`ADMINISTRATOR`)
-    to `OPERATOR` with an option URL parameter `privilegelevel`.
-    `ipmi://<host>:<port>?privilegelevel=OPERATOR`
 * Dell iDRAC
   * `idrac://` (or `idrac+http://` to disable TLS).
   * `idrac-virtualmedia://` to use virtual media instead of PXE
diff --git a/ironic-deployment/ironic/ironic.yaml b/ironic-deployment/ironic/ironic.yaml
index 788646a9..e2a88a88 100644
--- a/ironic-deployment/ironic/ironic.yaml
+++ b/ironic-deployment/ironic/ironic.yaml
@@ -4,10 +4,6 @@ metadata:
   name: capm3-ironic
 spec:
   replicas: 1
-  strategy:
-    # We cannot run Ironic with more than one replica at a time. The recreate
-    # strategy makes sure that the old pod is gone before a new is started.
-    type: Recreate
   selector:
     matchLabels:
       name: capm3-ironic
@@ -50,7 +46,7 @@ spec:
               valueFrom:
                  configMapKeyRef:
                   name: ironic-bmo-configmap
-                  key: RESTART_CONTAINER_CERTIFICATE_UPDATED
+                  key: RESTART_CONTAINER_CERTIFICATE_UPDATED 
         - name: ironic-api
           image: quay.io/metal3-io/ironic
           imagePullPolicy: Always
diff --git a/pkg/bmc/ipmi.go b/pkg/bmc/ipmi.go
index 98c283a8..ad8ce1e7 100644
--- a/pkg/bmc/ipmi.go
+++ b/pkg/bmc/ipmi.go
@@ -12,24 +12,11 @@ func init() {
 	RegisterFactory("libvirt", newIPMIAccessDetails, []string{})
 }
 
-func getPrivilegeLevel(rawquery string) string {
-	privilegelevel := "ADMINISTRATOR"
-	q, err := url.ParseQuery(rawquery)
-	if err != nil {
-		return privilegelevel
-	}
-	if val, ok := q["privilegelevel"]; ok {
-		return val[0]
-	}
-	return privilegelevel
-}
-
 func newIPMIAccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (AccessDetails, error) {
 	return &ipmiAccessDetails{
 		bmcType:                        parsedURL.Scheme,
 		portNum:                        parsedURL.Port(),
 		hostname:                       parsedURL.Hostname(),
-		privilegelevel:                 getPrivilegeLevel(parsedURL.RawQuery),
 		disableCertificateVerification: disableCertificateVerification,
 	}, nil
 }
@@ -38,7 +25,6 @@ type ipmiAccessDetails struct {
 	bmcType                        string
 	portNum                        string
 	hostname                       string
-	privilegelevel                 string
 	disableCertificateVerification bool
 }
 
@@ -75,11 +61,10 @@ func (a *ipmiAccessDetails) DisableCertificateVerification() bool {
 // the kernel and ramdisk locations).
 func (a *ipmiAccessDetails) DriverInfo(bmcCreds Credentials) map[string]interface{} {
 	result := map[string]interface{}{
-		"ipmi_port":       a.portNum,
-		"ipmi_username":   bmcCreds.Username,
-		"ipmi_password":   bmcCreds.Password,
-		"ipmi_address":    a.hostname,
-		"ipmi_priv_level": a.privilegelevel,
+		"ipmi_port":     a.portNum,
+		"ipmi_username": bmcCreds.Username,
+		"ipmi_password": bmcCreds.Password,
+		"ipmi_address":  a.hostname,
 	}
 
 	if a.disableCertificateVerification {
diff --git a/tools/run_local_ironic.sh b/tools/run_local_ironic.sh
index ff7b3713..4c97f2c1 100755
--- a/tools/run_local_ironic.sh
+++ b/tools/run_local_ironic.sh
@@ -84,17 +84,19 @@ IRONIC_INSPECTOR_VLAN_INTERFACES=${IRONIC_INSPECTOR_VLAN_INTERFACES}
 IPA_BASEURI=${IPA_BASEURI}
 EOF
 
+if [ "$IRONIC_TLS_SETUP" == "true" ]; then
 # shellcheck disable=SC2086
 cat << EOF | kubectl apply -f -
 apiVersion: v1
 data:
-  tls.crt: ${IRONIC_CA_CERT_B64:-""}
+  tls.crt: ${IRONIC_CA_CERT_B64}
 kind: Secret
 metadata:
    name: ironic-cacert
    namespace: capm3-system
 type: Opaque
 EOF
+fi
 
 sudo "${CONTAINER_RUNTIME}" pull "$IRONIC_IMAGE"
 sudo "${CONTAINER_RUNTIME}" pull "$IRONIC_INSPECTOR_IMAGE"


Test patch:
diff --git a/pkg/bmc/access_test.go b/pkg/bmc/access_test.go
index 517b2cca..2cc06102 100644
--- a/pkg/bmc/access_test.go
+++ b/pkg/bmc/access_test.go
@@ -678,25 +678,11 @@ func TestDriverInfo(t *testing.T) {
 			Scenario: "ipmi default port",
 			input:    "ipmi://192.168.122.1",
 			expects: map[string]interface{}{
-				"ipmi_port":       ipmiDefaultPort,
-				"ipmi_password":   "",
-				"ipmi_username":   "",
-				"ipmi_address":    "192.168.122.1",
-				"ipmi_verify_ca":  false,
-				"ipmi_priv_level": "ADMINISTRATOR",
-			},
-		},
-
-		{
-			Scenario: "ipmi setting privilege level",
-			input:    "ipmi://192.168.122.1?privilegelevel=OPERATOR",
-			expects: map[string]interface{}{
-				"ipmi_port":       ipmiDefaultPort,
-				"ipmi_password":   "",
-				"ipmi_username":   "",
-				"ipmi_address":    "192.168.122.1",
-				"ipmi_verify_ca":  false,
-				"ipmi_priv_level": "OPERATOR",
+				"ipmi_port":      ipmiDefaultPort,
+				"ipmi_password":  "",
+				"ipmi_username":  "",
+				"ipmi_address":   "192.168.122.1",
+				"ipmi_verify_ca": false,
 			},
 		},
 
diff --git a/pkg/provisioner/ironic/prepare_test.go b/pkg/provisioner/ironic/prepare_test.go
index 78d6b05c..bdf7c223 100644
--- a/pkg/provisioner/ironic/prepare_test.go
+++ b/pkg/provisioner/ironic/prepare_test.go
@@ -1,7 +1,6 @@
 package ironic
 
 import (
-	"net/url"
 	"testing"
 	"time"
 
@@ -16,28 +15,7 @@ import (
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/testserver"
 )
 
-type RAIDTestBMC struct{}
-
-func (r *RAIDTestBMC) Type() string                                          { return "raid-test" }
-func (r *RAIDTestBMC) NeedsMAC() bool                                        { return false }
-func (r *RAIDTestBMC) Driver() string                                        { return "raid-test" }
-func (r *RAIDTestBMC) DisableCertificateVerification() bool                  { return false }
-func (r *RAIDTestBMC) DriverInfo(bmc.Credentials) (i map[string]interface{}) { return }
-func (r *RAIDTestBMC) BootInterface() string                                 { return "" }
-func (r *RAIDTestBMC) ManagementInterface() string                           { return "" }
-func (r *RAIDTestBMC) PowerInterface() string                                { return "" }
-func (r *RAIDTestBMC) RAIDInterface() string                                 { return "" }
-func (r *RAIDTestBMC) VendorInterface() string                               { return "" }
-func (r *RAIDTestBMC) SupportsSecureBoot() bool                              { return false }
-func (r *RAIDTestBMC) BuildBIOSSettings(fwConf *metal3v1alpha1.FirmwareConfig) ([]map[string]string, error) {
-	return nil, nil
-}
-
 func TestPrepare(t *testing.T) {
-	bmc.RegisterFactory("raid-test", func(u *url.URL, dcv bool) (bmc.AccessDetails, error) {
-		return &RAIDTestBMC{}, nil
-	}, []string{})
-
 	nodeUUID := "33ce8659-7400-4c68-9535-d10766f07a58"
 	cases := []struct {
 		name                 string
@@ -180,7 +158,7 @@ func TestPrepare(t *testing.T) {
 			host.Status.Provisioning.ID = nodeUUID
 			prepData := provisioner.PrepareData{}
 			if tc.existRaidConfig {
-				host.Spec.BMC.Address = "raid-test://test.bmc/"
+				host.Spec.BMC.Address = "idrac://test.bmc/"
 				prepData.RAIDConfig = &metal3v1alpha1.RAIDConfig{
 					HardwareRAIDVolumes: []metal3v1alpha1.HardwareRAIDVolume{
 						{