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

Adding sample perl and shell script for CA migration api's #65

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bkharpuse
Copy link

No description provided.

@bkharpuse bkharpuse requested review from riteshopsranjan and malhotrag and removed request for malhotrag June 1, 2020 11:52
@bkharpuse bkharpuse force-pushed the feature/CA_migration_scripts branch from 5989f94 to 4dc9d9f Compare June 2, 2020 17:33
Copy link
Collaborator

@riteshopsranjan riteshopsranjan left a comment

Choose a reason for hiding this comment

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

Looks good

Comment on lines +107 to +114
request_body="{"
request_body="${request_body}\"data\": {"
request_body="${request_body}\"type\": \"nbcaMigrationActivateRequest\","
request_body="${request_body}\"attributes\": {"
if [ $force == 1 ]; then
request_body="${request_body}\"force\" : \"true\""
fi
request_body="${request_body}}}}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you already know you have jq, you may as well use it to generate syntactically valid JSON without a bunch of string concatenations.

Suggested change
request_body="{"
request_body="${request_body}\"data\": {"
request_body="${request_body}\"type\": \"nbcaMigrationActivateRequest\","
request_body="${request_body}\"attributes\": {"
if [ $force == 1 ]; then
request_body="${request_body}\"force\" : \"true\""
fi
request_body="${request_body}}}}"
request_body=$(jq -n --argjson force "$force" '{
data: {
type: "nbcaMigrationActivateRequest",
attributes: {
force: (if $force == 1 then "true" else "false"),
},
}
}')

request_body="${request_body}\"data\": {"
request_body="${request_body}\"type\": \"nbcaMigrationActivateRequest\","
request_body="${request_body}\"attributes\": {"
if [ $force == 1 ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

The == operator is a Bash extension. Standard Posix uses = for comparisons.


uri="$basepath/login"

data=$(jq --arg name $login_username --arg pass $login_password --arg dname $login_domainname --arg dtype $login_domaintype \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Quote the shell parameters. No idea what crazy characters might be in any of those.

Suggested change
data=$(jq --arg name $login_username --arg pass $login_password --arg dname $login_domainname --arg dtype $login_domaintype \
data=$(jq --arg name "$login_username" --arg pass "$login_password" --arg dname "$login_domainname" --arg dtype "$login_domaintype" \

Comment on lines +62 to +63
-force|-f)
force=1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since -force doesn't consume $2, it must be the final parameter on the command line. I realize it's also listed last in the synopsis, but that's a really subtle detail. Most programs will accept options in any order, and indeed, this program accepts all the other options in any order.

You can fix this by using shift in each case branch according to how many arguments each branch consumes, instead of assuming each branch always consumes two arguments.

showHelp
fi

if [ "${login_domaintype^^}" = "WINDOWS" ] || [ "${login_domaintype^^}" = "NT" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

The shebang line says this runs as /bin/sh, but the ^^ parameter-expansion modifier is a Bash extension. It's not in standard sh. You can use something like tr '[:lower:]' '[:upper:]' to convert a string to uppercase.

request_body="${request_body}\"data\": {"
request_body="${request_body}\"type\": \"initiateCAMigrationRequest\","
request_body="${request_body}\"attributes\": {"
request_body="${request_body}\"keySize\" : \"${keysize}\""
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think keySize is documented to be an integer, but this sends it as a string.

request_body="${request_body}\"type\": \"nbcaMigrationCompleteRequest\","
request_body="${request_body}\"attributes\": {"
if [ $force == 1 ]; then
request_body="${request_body}\"force\" : \"true\""
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think force is documented to be a Boolean, but this sends it as a string.

Comment on lines +133 to +139
- `perl initiate-migration.pl -nbmaster <master_server> -login_username <login_username> -login_password <login_password> [-login_domainname <login_domain_name> -login_domaintype <domain_type>] -keysize <keysize> [-reason <reason>] [--verbose]`

- Use the following command to activate the new NetBackup CA on your NetBackup Master server:
- `perl activate_migration.pl -nbmaster <master_server> -login_username <login_username> -login_password <login_password> [-login_domainname <login_domain_name> -login_domaintype <domain_type>] [-reason <reason>] [--force] [--verbose]`

- Use the following command to complete the NetBackup CA migration on your NetBackup Master server:
- `perl complete_migration.pl -nbmaster <master_server> -login_username <login_username> -login_password <login_password> [-login_domainname <login_domain_name> -login_domaintype <domain_type>] [-reason <reason>] [--force] [--verbose]`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why using single dash for some options and double dash for others?

my $base_url = "$protocol://$fqdn_hostname:$port/netbackup";

my $token;
if (defined $login_domainname && defined $login_domaintype) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't line 52 already check that these have values?

Comment on lines +159 to +164
if (@argument_list == 4) {
$reason = $argument_list[2];
$force = $argument_list[3];
} elsif (@argument_list == 3) {
$force = $argument_list[2];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Parameters 2 and 3 are both listed as optional. If I provide parameter 2 and not parameter 3, then the length of the array will be 3, but this will assign the value to $force instead of to $reason. I think you can simplify the whole thing with a single line:

my ($base_url, $token, $reason, $force) = @_;

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.

3 participants