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

Allow username detection on older Cisco ios versions #1121

Merged

Conversation

dlundgren
Copy link
Contributor

This switches to the older | include syntax, and allows usernames with password to be detected.

This switches to the older `| include` syntax, and allows usernames with `password` to be detected.
@coveralls
Copy link

coveralls commented Feb 7, 2020

Coverage Status

Coverage remained the same at 0.0% when pulling 75a4f6f on dlundgren:dl/ios-username-detection into d0df828 on napalm-automation:develop.

@dlundgren
Copy link
Contributor Author

It looks like while this will work for older ios versions, as I have available, it may not work properly for newer versions. I'm evaluating if it's plausible to support detection of section vs include.

@ktbyers
Copy link
Contributor

ktbyers commented Feb 10, 2020

@dlundgren Can you expand a bit on what works on older IOS versions and what doesn't work on newer IOS versions (as it is a bit unclear to me the reference of this here). In other words, a bit unclear to me if you are talking about the original problem or the fix.

@dlundgren
Copy link
Contributor Author

Older IOS does not support section, but it does support include.

For example, I have a 3750G with IOS 12.2(44)SE12 and using | include username allows napalm to detect the users.

On newer IOS with support for ssh keys, include fails to load the key-hash for a user, but section does include the key-hash.

@ktbyers
Copy link
Contributor

ktbyers commented Feb 10, 2020

Yeah, I guess you could do a PR and try the newer form first i.e. | section and then if Invalid command comes back try the | include solution.

@mirceaulinic
Copy link
Member

Hey @dlundgren - have you seen Kirk's suggestion?

@dlundgren
Copy link
Contributor Author

I haven't had time to fix this up with his suggestions. I will try to look at this over the next week.

@dlundgren
Copy link
Contributor Author

I've updated to check the command output by looking for Invalid input detected in the output of the command using | section username, then falling back to trying | include username

@mirceaulinic mirceaulinic added this to the 3.0.2 milestone Jul 10, 2020
Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

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

This looks good to me now!

@mirceaulinic mirceaulinic requested a review from ktbyers July 10, 2020 10:24
@mirceaulinic mirceaulinic merged commit 1d9fd36 into napalm-automation:develop Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants