Skip to content

Commit

Permalink
Code-review fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
Tomasz Mieszkowski committed Jun 13, 2016
1 parent 7be5ca9 commit 37c3cc9
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 37 deletions.
8 changes: 4 additions & 4 deletions bindata.go

Large diffs are not rendered by default.

24 changes: 17 additions & 7 deletions bundled_scripts/idrac.py
@@ -1,9 +1,9 @@
#!/usr/bin/env python

import argparse
import json
import os
import re
import sys
import uuid
from xml.etree import ElementTree as ET

Expand Down Expand Up @@ -341,14 +341,24 @@ def idrac_device_info(idrac_manager):
return device_info


def scan(host):
idrac_manager = IDRAC(host, USER, PASS)
def scan(host, user, password):
if host == "":
raise IdracError("No IP address to scan has been provided.")
if user == "":
raise IdracError("No management username has been provided.")
if host == "":
raise IdracError("No management password has been provided.")
idrac_manager = IDRAC(host, user, password)
device_info = idrac_device_info(idrac_manager)
print(json.dumps(device_info))


if __name__ == '__main__':
parser = argparse.ArgumentParser()
parser.add_argument('host', nargs=1, help='host to scan')
args = parser.parse_args()
scan(args.host[0])
host = os.environ.get('IP_TO_SCAN', "")
user = os.environ.get('MANAGEMENT_USER_NAME', "")
password = os.environ.get('MANAGEMENT_USER_PASSWORD', "")
try:
scan(host, user, password)
except IdracError as e:
print(e.args[0])
sys.exit(1)
26 changes: 14 additions & 12 deletions bundled_scripts/ilo.py
@@ -1,6 +1,5 @@
#!/usr/bin/env python

import argparse
import json
import os
import sys
Expand All @@ -9,9 +8,6 @@
import hpilo


USER = os.environ['MANAGEMENT_USER_NAME']
PASS = os.environ['MANAGEMENT_USER_PASSWORD']

MAC_PREFIX_BLACKLIST = [
'505054', '33506F', '009876', '000000', '00000C', '204153', '149120',
'020054', 'FEFFFF', '1AF920', '020820', 'DEAD2C', 'FEAD4D',
Expand Down Expand Up @@ -52,8 +48,8 @@ def normalize_mac_address(mac_address):
return mac_address


def get_ilo_instance(host):
ilo = hpilo.Ilo(hostname=host, login=USER, password=PASS, port=443)
def get_ilo_instance(host, user, password):
ilo = hpilo.Ilo(hostname=host, login=user, password=password)
return ilo


Expand Down Expand Up @@ -223,19 +219,25 @@ def ilo_device_info(ilo_manager, ilo_version):
return device_info


def scan(host):
ilo_manager = get_ilo_instance(host)
def scan(host, user, password):
if host == "":
raise IloError("No IP address to scan has been provided.")
if user == "":
raise IloError("No management username has been provided.")
if host == "":
raise IloError("No management password has been provided.")
ilo_manager = get_ilo_instance(host, user, password)
ilo_version = get_ilo_version(ilo_manager)
device_info = ilo_device_info(ilo_manager, ilo_version)
print(json.dumps(device_info))


if __name__ == '__main__':
parser = argparse.ArgumentParser()
parser.add_argument('host', nargs=1, help='host to scan')
args = parser.parse_args()
host = os.environ.get('IP_TO_SCAN', "")
user = os.environ.get('MANAGEMENT_USER_NAME', "")
password = os.environ.get('MANAGEMENT_USER_PASSWORD', "")
try:
scan(args.host[0])
scan(host, user, password)
except (IloError, hpilo.IloCommunicationError) as e:
print(e.args[0])
sys.exit(1)
16 changes: 10 additions & 6 deletions scan.go
Expand Up @@ -44,14 +44,15 @@ func NewScript(fileName, cfgDir string) (Script, error) {

// Run launches a scan Script on a given address (at this moment, only IPs are fully
// supported).
func (s Script) Run(addr Addr, cfg *Config) (*ScanResult, error) {
func (s Script) Run(addrToScan Addr, cfg *Config) (*ScanResult, error) {
var res ScanResult
var err error
cmd := execCommand(s.LocalPath, string(addr))
// cmd.Env won't be empty during some tests (see GetHelperCommand),
// so this check is necessary to preserve it.
cmd := execCommand(s.LocalPath)
// This condition will be false only during some tests (see GetHelperCommand),
// and in such case, we need to preserve cmd.Env contents, hence this check
// (i.e., prepareEnv should only be launched when cmd.Env is empty).
if len(cmd.Env) == 0 {
cmd.Env = prepareEnv(os.Environ(), cfg)
cmd.Env = prepareEnv(os.Environ(), addrToScan, cfg)
}
output, err := cmd.CombinedOutput()
if err != nil {
Expand All @@ -64,20 +65,23 @@ func (s Script) Run(addr Addr, cfg *Config) (*ScanResult, error) {

// prepareEnv is a helper function for Script.Run. It modifies the environment that
// should be used for executing given Script.
func prepareEnv(oldEnv []string, cfg *Config) (newEnv []string) {
func prepareEnv(oldEnv []string, addrToScan Addr, cfg *Config) (newEnv []string) {
for _, e := range oldEnv {
pair := strings.Split(e, "=")
switch {
case pair[0] == "MANAGEMENT_USER_NAME":
continue
case pair[0] == "MANAGEMENT_USER_PASSWORD":
continue
case pair[0] == "IP_TO_SCAN":
continue
default:
newEnv = append(newEnv, e)
}
}
newEnv = append(newEnv, fmt.Sprintf("MANAGEMENT_USER_NAME=%s", cfg.ManagementUserName))
newEnv = append(newEnv, fmt.Sprintf("MANAGEMENT_USER_PASSWORD=%s", cfg.ManagementUserPassword))
newEnv = append(newEnv, fmt.Sprintf("IP_TO_SCAN=%s", addrToScan))
return newEnv
}

Expand Down
19 changes: 11 additions & 8 deletions scan_test.go
Expand Up @@ -28,9 +28,10 @@ func TestRunHelperProcess(t *testing.T) {

func TestPrepareEnv(t *testing.T) {
var cases = map[string]struct {
oldEnv []string
config *Config
want []string
oldEnv []string
config *Config
addrToScan Addr
want []string
}{
"#0 Existing Cmd.Env shouldn't be destroyed": {
[]string{"GO_WANT_HELPER_PROCESS=1"},
Expand All @@ -41,22 +42,24 @@ func TestPrepareEnv(t *testing.T) {
ManagementUserName: "some_user",
ManagementUserPassword: "some_password",
},
[]string{"GO_WANT_HELPER_PROCESS=1", "MANAGEMENT_USER_NAME=some_user", "MANAGEMENT_USER_PASSWORD=some_password"},
Addr("10.20.30.40"),
[]string{"GO_WANT_HELPER_PROCESS=1", "MANAGEMENT_USER_NAME=some_user", "MANAGEMENT_USER_PASSWORD=some_password", "IP_TO_SCAN=10.20.30.40"},
},
"#1 Existing management user/pass should be overwritten": {
[]string{"MANAGEMENT_USER_NAME=old_user", "MANAGEMENT_USER_PASSWORD=old_password"},
"#1 Existing management user/pass/IP should be overwritten": {
[]string{"MANAGEMENT_USER_NAME=old_user", "MANAGEMENT_USER_PASSWORD=old_password", "IP_TO_SCAN=11.22.33.44"},
&Config{
ClientTimeout: 10,
RalphAPIURL: "http://localhost:8080/api",
RalphAPIKey: "abcdefghijklmnopqrstuwxyz0123456789ABCDE",
ManagementUserName: "some_user",
ManagementUserPassword: "some_password",
},
[]string{"MANAGEMENT_USER_NAME=some_user", "MANAGEMENT_USER_PASSWORD=some_password"},
Addr("10.20.30.40"),
[]string{"MANAGEMENT_USER_NAME=some_user", "MANAGEMENT_USER_PASSWORD=some_password", "IP_TO_SCAN=10.20.30.40"},
},
}
for tn, tc := range cases {
got := prepareEnv(tc.oldEnv, tc.config)
got := prepareEnv(tc.oldEnv, tc.addrToScan, tc.config)
if !TestEqStr(got, tc.want) {
t.Errorf("%s\n got: %v\nwant: %v", tn, got, tc.want)
}
Expand Down

0 comments on commit 37c3cc9

Please sign in to comment.