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.

Sign up for GitHub

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

Jump to bottom

!feature(util/commit_push): Refactor Commit/Push script #54

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

!feature(util/commit_push): Refactor Commit/Push script #54

wants to merge 1 commit into from

Conversation

pimielowski
Copy link
Contributor

@pimielowski pimielowski commented Nov 13, 2023 •
edited
Loading

Description

This PR largely improve current commit/push script logic. We introduce new options plus refactor current ones to fit Go standard.

Motivation and Context

We was focused on fixed current code and improve its capabilities. This version of the code deduplicate pieces that we can merge into one function. Also we introduce locking mechanism based on Pango repository PR (PaloAltoNetworks/pango#113 and PaloAltoNetworks/pango#111).
In proposed way we can use this script to lock Panorama config for provided Device Group and do changes in it. This feature can improve usage of the script in the CI/CD and highly automated environment.

For testing purposes you can use forked version with all feature/fixes added to Pango https://github.com/pimielowski/pango

Example usages:

╰─± go run push_commit.go -config ../../basic_configuration_example/credentials.json -admins panadmin -mode lock -deviceGroup "pimielow_test_dg" -user panadmin
2023/11/13 14:47:55.913128 Configure PAN-OS Client.
2023/11/13 14:47:58.311205 Check for Commit lock for dg: pimielow_test_dg
2023/11/13 14:47:58.571415 Set mode to lock for dg: pimielow_test_dg
2023/11/13 14:47:58.809410 Config for dg:pimielow_test_dg for user:panadmin is locked!
╰─± go run push_commit.go -config ../../basic_configuration_example/credentials.json -admins panadmin -mode unlock -devi
ceGroup "pimielow_test_dg" -user panadmin
2023/11/13 14:49:08.487720 Configure PAN-OS Client.
2023/11/13 14:49:09.452889 Check for Commit lock for dg: pimielow_test_dg
2023/11/13 14:49:10.133051 Config for dg:pimielow_test_dg for user:panadmin was unlocked succesfully!
╰─± go run push_commit.go -config ../../basic_configuration_example/credentials.json -admins panadmin -mode lock -device
Group "pimielow_test_dg" -user panadmin
2023/11/13 14:50:24.696277 Configure PAN-OS Client.
2023/11/13 14:50:25.668988 Check for Commit lock for dg: pimielow_test_dg
2023/11/13 14:50:25.884922 Set mode to lock for dg: pimielow_test_dg
2023/11/13 14:50:26.111508 Waiting: Lock is acquire, waiting for release...
(...)
2023/11/13 14:52:52.416245 Waiting: Lock is acquire, waiting for release...
2023/11/13 14:52:57.417606 Error: Config lock was not acquired after 0 retries
exit status 1
╰─± go run push_commit.go -config ../../basic_configuration_example/credentials.json -admins panadmin -mode lock -deviceGroup "pimielow_test_dg" -user panadmin
2023/11/13 14:53:50.349760 Configure PAN-OS Client.
2023/11/13 14:53:51.355638 Check for Commit lock for dg: pimielow_test_dg
2023/11/13 14:53:51.568765 Waiting: Lock is acquire, contact with Admin panadmin for release it.
(...)
2023/11/13 14:56:23.480801 Waiting: Lock is acquire, contact with Admin panadmin for release it.
2023/11/13 14:56:28.481344 Error: another admin is holding commit lock or error requesting commit lock state.
exit status 1

How Has This Been Tested?

We run multiple combination of this script to found any edge cases that we can.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

@pimielowski pimielowski added the enhancement New feature or request label Nov 13, 2023
@pimielowski pimielowski self-assigned this Nov 13, 2023
@pimielowski pimielowski linked an issue Nov 13, 2023 that may be closed by this pull request
feat(pango tools): upgrade commit and push behaviour #14
Open
@pimielowski pimielowski changed the title !(feat/util): Refactor Commit/Push script !(feature/util): Refactor Commit/Push script Nov 14, 2023
@pimielowski pimielowski changed the title !(feature/util): Refactor Commit/Push script !feature(util/commit_push): Refactor Commit/Push script Nov 14, 2023
sebastianczech
sebastianczech approved these changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@sebastianczech sebastianczech left a comment

Choose a reason for hiding this comment

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

Great job 👍

utils/panorama_commit_push.go
}

if mode == "lock" {
log.Printf("Set mode to lock for dg: %s \n", deviceGroup)
Copy link
Contributor

Choose a reason for hiding this comment

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

Function deviceLocking is already quite long , so what do you think about extracting whole logic after if mode == "lock" into dedicated function e.g. lockConfig() ?

utils/panorama_commit_push.go
}
}

if mode == "unlock" {
dgLocks, _ := panorama.Client.ShowConfigLocks(deviceGroup)
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar like previous comment - function deviceLocking is already quite long , so what do you think about extracting whole logic after if mode == "unlock" into dedicated function e.g. unlockConfig()?

utils/panorama_commit_push.go
}

log.Printf("Check for Commit lock for dg: %s", deviceGroup)
for maxRetries > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could extract whole part with for maxRetries > 0 into dedicated function e.g. checkCommitLock()?

@sebastianczech sebastianczech mentioned this pull request May 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebastianczech sebastianczech sebastianczech approved these changes

@migara migara Awaiting requested review from migara

@michalbil michalbil Awaiting requested review from michalbil

At least 1 approving review is required to merge this pull request.

Assignees

@pimielowski pimielowski

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat(pango tools): upgrade commit and push behaviour
2 participants
@pimielowski @sebastianczech