Skip to content

Conversation

BrianSantivanez
Copy link

Issue: #1942
Observations: sshkeys and ssl features were merged into security feature, and core test were updated to work with commands names that start with other command names (securitygroup and security), before this upgrade if exists a command that starts with other command name this test fails

slcli security -h
Usage: slcli security [OPTIONS] COMMAND [ARGS] ...

        SSH Keys and SSL Certificates.

┌────┬────────┬─────────────────────────────┐
│ -h │ --help │ Show this message and exit. │
└────┴────────┴─────────────────────────────┘

Commands:
  cert-add       Add and upload SSL certificate details.
  cert-download  Download SSL certificate and key file.
  cert-edit      Edit SSL certificate.
  cert-list      List SSL certificates.
  cert-remove    Remove SSL certificate.
  sshkey-add     Add a new SSH key.
  sshkey-edit    Edits an SSH key.
  sshkey-list    List SSH keys.
  sshkey-print   Prints out an SSH key to the screen.
  sshkey-remove  Permanently removes an SSH key.

…ands that name starts with other command names
@BrianSantivanez BrianSantivanez self-assigned this May 16, 2023
@BrianSantivanez BrianSantivanez linked an issue May 16, 2023 that may be closed by this pull request
@allmightyspiff
Copy link
Member

@ramkishor-ch when testing this, please make sure the slcli sshkeys and slcli ssl commands work as expected as well.

@ramkishor-ch
Copy link
Contributor

ramkishor-ch commented May 17, 2023

Hi @allmightyspiff,
When I executed the slcli sshkey and slcli ssl commands that were generated an ERROR,
not working as expected

Hi @BrianSantivanez,
After checkout of BrianSantivanez-issue1942 changes into local system.
When I tried to run With Fix of #1951

./slcli ssl --help
Usage: slcli [OPTIONS] COMMAND [ARGS] ...
Try 'slcli --help' for help.
Error: No such command 'ssl'.

./slcli sshkey --help
Usage: slcli [OPTIONS] COMMAND [ARGS] ...
Try 'slcli --help' for help.
Error: No such command 'sshkey'.

Can you please fix this issue.

Thanks,
Ramkishor Chaladi.

@BrianSantivanez
Copy link
Author

Hi @ramkishor-ch, the errors you mentioned are expected because slcli sshkey and slcli ssl were removed and all their commands were added in slcli security

image

If you want to run some sshkey or ssl command you have to do from slcli security

image

@ramkishor-ch
Copy link
Contributor

Hi @BrianSantivanez,

I already knew that slcli sshkey and slcli ssl commands were moved under slcli security, but I have already discussed the same doubt on yesterday with @allmightyspiff.

@allmightyspiff was told that " make sure still old commands: slcli sshkey and slcli ssl should work though that were added into slcli security because anyone using the slcli sshkey and slcli ssl still be able to use the old commands so it doesn't break their automation and if the commands DON'T exist, thats a problem that needs to be fixed ".

Can you please fix the issue.

Thanks,
Ramkishor Chaladi.

@allmightyspiff
Copy link
Member

@BrianSantivanez There should still be an alias for slcli sshkey and slcli ssl for users who have these commands in some automation. I don't want to break existing features.

Moving the commands into a single security command is good, but we should add alias routes for them as well. Thanks

Basically just add these to routes.py courtesy of Ramkishor

    ('sshkey', 'SoftLayer.CLI.sshkey'),
    ('sshkey:add', 'SoftLayer.CLI.security.sshkey_add:cli'),
    ('sshkey:remove', 'SoftLayer.CLI.security.sshkey_remove:cli'),
    ('sshkey:edit', 'SoftLayer.CLI.security.sshkey_edit:cli'),
    ('sshkey:list', 'SoftLayer.CLI.security.sshkey_list:cli'),
    ('sshkey:print', 'SoftLayer.CLI.security.sshkey_print:cli'),
    ('ssl', 'SoftLayer.CLI.ssl'),
    ('ssl:add', 'SoftLayer.CLI.security.cert_add:cli'),
    ('ssl:download', 'SoftLayer.CLI.security.cert_download:cli'),
    ('ssl:edit', 'SoftLayer.CLI.security.cert_edit:cli'),
    ('ssl:list', 'SoftLayer.CLI.security.cert_list:cli'),
    ('ssl:remove', 'SoftLayer.CLI.security.cert_remove:cli'),

@ramkishor-ch
Copy link
Contributor

Hi,

I have "tested the changes".
Please find the screenshots for reference.

Screen Shot: 1
Screenshot 2023-05-19 at 4 30 01 PM

Screen Shot: 2
Screenshot 2023-05-19 at 4 31 28 PM

Screen Shot: 3
Screenshot 2023-05-19 at 4 32 18 PM

Screen Shot: 4
Screenshot 2023-05-19 at 4 33 05 PM

Screen Shot: 5
Screenshot 2023-05-19 at 4 33 41 PM

Screen Shot: 6
Screenshot 2023-05-19 at 4 34 07 PM

Thanks,
Ramkishor Chaladi.

Copy link
Contributor

@ramkishor-ch ramkishor-ch left a comment

Choose a reason for hiding this comment

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

Hi,
I have "tested the changes".
Approving the Pull Request.
Thanks,
Ramkishor Chaladi.

@allmightyspiff allmightyspiff merged commit 926e24b into softlayer:master May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

New Command: slcli security
3 participants