Skip to content
This repository has been archived by the owner on Sep 25, 2024. It is now read-only.

(repost) - Obtain version on class init, allow compatibility with 2.x and 3.x APIs. #168

Closed
wants to merge 9 commits into from

Conversation

jasonbarbee
Copy link
Contributor

Per comments of PR #151, @falkowich you asked me to recommit. I re-synced with master, and tried to re-commit. Pardon the "new line" I couldn't seem to match that, I didn't want to change any code there, it seems some kind of editor issue.

@JonasKs
Copy link
Collaborator

JonasKs commented Jun 19, 2021

This should be optional. We use ISE through APIs and have no desire to do an extra API call for every request that hit our API (new class is instanced for each request)

ise.py Show resolved Hide resolved
@jasonbarbee
Copy link
Contributor Author

After thinking on this more... I split up the code into 2 optional parameters, and went ahead and added the failover support to this PR (hope that's not too much, but it seemed be related so we could all see how it looks)

Version Discovery - on by default, toggle by setting the version on init:
Default version variable, set to '', if it's blank we discover it, if it is set on initialization - we use that, and skip version discovery. The user can choose to lock in a version, or if it is blank - it's automatic discovery of the version.

HA Discovery off by default, toggle on with a boolean variable:
parameter - enable_failover (true/false) - takes care of the HA discovery features. Default to false - because not everyone needs this.

Possible improvement to do is that the library only takes 1 IP today. The failover discovery will work as long as that IP has either Active or Standby role. If it's standby, it will find the other primary node, and assign it to the class on init, so all future calls will use the discovered IP. I could just add a new parameter - secondary_ise_node, or ise_node2, and run queries there if the primary is offline.

Thoughts?

ise.py Outdated
print("Error: Connection Error to Secondary ISE Node.")
raise
except requests.exceptions.RequestException as e:
print("Error: Connection Error to Secondary ISE Node.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

All prints should be converted into logging statements.

import logging

log = logging.getLogger('ise')
log.debug(…)
log.warning(…)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added logging to replace prints.

@falkowich
Copy link
Owner

I will check this out as soon as #164 is done

And I am sorry for taking so much time with this..

--
Regards Falk

@falkowich falkowich added this to the 0.2.0 milestone Sep 30, 2021
falkowich added a commit that referenced this pull request Oct 2, 2021
@falkowich
Copy link
Owner

If there is a dedicated ERS user this will not work i guess?

How is the best way to fix that?
Another user_pass dict so that we can use a read_only user that can scrape the frontend?

Beq cisco wanted a namechange I had to do #164.
So your merge is in bransh #168

--
Regards Falk

@jasonbarbee
Copy link
Contributor Author

Yeah, they need both MnT and ERS group rights in current code, which may be a little unexpected. I have an idea.

I just found an ISE ERS API call that will let us query if the ERS node is primary or not, and even see the secondary. I can move this feature down to the ERS only permission.
https://developer.cisco.com/docs/identity-services-engine/3.0/#!node-details/get-by-id

I can build changes to use ERS only, and fix the conflict - any other changes requested?

@falkowich
Copy link
Owner

@jasonbarbee That would be great!
Then we could get this into 0.3.0 :)

--
Regards Falk

@falkowich
Copy link
Owner

Any more information on this, I'm trying to clean up some PR's before 0.3.
So if someone have more information on this I'm all ears :)

@falkowich
Copy link
Owner

Closing this.
If it's still up for merge, please renew the PR.

Really sorry for the trouble.

@falkowich falkowich closed this May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants