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 instance -> custom_headers and instance -> api_path #239

Merged

Conversation

augustinosusky
Copy link
Contributor

SUMMARY

Adding functionality to add custom headers under instance parameter. This may be required if you are using API gateways intercepting API requests and then forwarding requests to the real endpoint.
Adding functionality to add custom api path in case it differs from the usual "api/now" path. For example if API gateway needs this.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

adding instance->custom_headers and instance->api_path parameters. This affects arguments.py, client.py, table.py and attachment.py in plugins/module_utils folder.

ADDITIONAL INFORMATION

Example call for configuration item:

- hosts: localhost
  gather_facts: no
  tasks:
    - name: Get CI
      servicenow.itsm.configuration_item_info:
        instance:
          host: https://apigateway.example.com
          username: automation_user
          password: xyz
          custom_headers: { X-APIGW-APIKEY: xyz }
          api_path: "servicenow/v1/now"

        sys_class_name: cmdb_ci_server
        sysparm_query: "fqdn=myserver.exmple.com"
      register: result

    - name: debug
      debug:
        msg: "{{ result }}"

@softwarefactory-project-zuul
Copy link

Build succeeded.

✔️ build-ansible-collection SUCCESS in 3m 19s
✔️ ansible-test-sanity-docker-devel SUCCESS in 7m 28s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 6m 14s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 9m 29s
✔️ ansible-test-sanity-docker-stable-2.10 SUCCESS in 8m 16s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 9m 08s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 6m 44s
✔️ ansible-test-units-servicenow-itsm-python27 SUCCESS in 4m 41s
✔️ ansible-test-units-servicenow-itsm-python36 SUCCESS in 5m 21s
✔️ ansible-test-units-servicenow-itsm-python37 SUCCESS in 4m 50s
✔️ ansible-test-units-servicenow-itsm-python38 SUCCESS in 4m 40s
✔️ ansible-galaxy-importer SUCCESS in 3m 39s

@augustinosusky augustinosusky marked this pull request as draft April 26, 2023 05:04
@augustinosusky augustinosusky marked this pull request as ready for review April 26, 2023 05:04
* fixing default api_path as it should be api/now
* improving documentation, changing _path functions to be backward compatible with python 2.*
* fixing deviations from pep8 standards
* adding default value setting into mocker for the Client as otherwise it was not picking up default value
@softwarefactory-project-zuul
Copy link

@Akasurde
Copy link
Member

recheck

@softwarefactory-project-zuul
Copy link

@Akasurde
Copy link
Member

@augustinosusky Could you please update/fix these CI failures? Thanks

@augustinosusky
Copy link
Contributor Author

augustinosusky commented Jul 25, 2023

@augustinosusky Could you please update/fix these CI failures? Thanks

Hello @Akasurde ,
I will check it and try to resolve it. I will also check and remove documentation updates since they are generated automatically.

Thank you for addressing this.

@softwarefactory-project-zuul
Copy link

@softwarefactory-project-zuul
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/ansible-collections/servicenow.itsm for 239,6a20f08d501558e3a5aa2bdc4ab50f5f1e80f446

@softwarefactory-project-zuul
Copy link

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/5002ad2331f54663a5273432c225fb92

✔️ build-ansible-collection SUCCESS in 7m 45s
✔️ ansible-test-sanity-docker-devel SUCCESS in 12m 04s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 41s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 17m 26s
✔️ ansible-test-sanity-docker-stable-2.10 SUCCESS in 9m 21s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 17m 58s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 12m 35s
✔️ ansible-test-units-servicenow-itsm-python27 SUCCESS in 4m 22s
✔️ ansible-test-units-servicenow-itsm-python36 SUCCESS in 5m 48s
✔️ ansible-test-units-servicenow-itsm-python37 SUCCESS in 6m 16s
ansible-test-units-servicenow-itsm-python38 RETRY_LIMIT in 5m 46s
✔️ ansible-galaxy-importer SUCCESS in 3m 17s

@augustinosusky
Copy link
Contributor Author

Hello @Akasurde,
It seems that the restarted build finished well now (although the log shows that there were many retries). Thank you for your help.
Is there anything else expected from me or is all ok now from pull request perspective?
Thanks again!
Augustin

@augustinosusky
Copy link
Contributor Author

Hello @Akasurde ,
is there something I am still missing? Or we can proceed?
Thank you!
Augustin

Copy link
Member

@Akasurde Akasurde left a comment

Choose a reason for hiding this comment

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

Minor change required.

plugins/doc_fragments/instance.py Outdated Show resolved Hide resolved
plugins/doc_fragments/instance.py Outdated Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link

@augustinosusky
Copy link
Contributor Author

augustinosusky commented Sep 1, 2023

Again framework errors:
2.9 - https://ansible.softwarefactory-project.io/zuul/build/6b37af9767f54107ab292140c73f7cb8

Collecting astroid==2.3.3 (from -c /root/ansible/test/lib/ansible_test/_data/requirements/constraints.txt (line 54))
WARNING: Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'ReadTimeoutError("HTTPConnectionPool(host='172.17.0.2', port=3141): Read timed out. (read timeout=15)",)': /root/pypi/+simple/astroid/

2.10 - https://ansible.softwarefactory-project.io/zuul/build/82fd369988984f889762b9ec430616c0

Collecting antsibull-changelog==0.9.0 (from -r /root/ansible/test/lib/ansible_test/_data/requirements/sanity.changelog.txt (line 2))
WARNING: Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'ReadTimeoutError("HTTPConnectionPool(host='172.17.0.2', port=3141): Read timed out. (read timeout=15)",)': /root/pypi/+simple/antsibull-changelog/

I will add some empty commit to start it again.

@softwarefactory-project-zuul
Copy link

@Akasurde
Copy link
Member

Akasurde commented Sep 1, 2023

recheck

@softwarefactory-project-zuul
Copy link

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/bbe7a0b158104ef4bf3cb60a4484574f

✔️ build-ansible-collection SUCCESS in 6m 42s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 08s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 54s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 26s
✔️ ansible-test-sanity-docker-stable-2.10 SUCCESS in 10m 54s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 13m 36s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 26s
✔️ ansible-test-units-servicenow-itsm-python27 SUCCESS in 5m 58s
✔️ ansible-test-units-servicenow-itsm-python36 SUCCESS in 4m 58s
✔️ ansible-test-units-servicenow-itsm-python37 SUCCESS in 5m 36s
✔️ ansible-test-units-servicenow-itsm-python38 SUCCESS in 5m 29s
✔️ ansible-galaxy-importer SUCCESS in 4m 16s

@Akasurde
Copy link
Member

Akasurde commented Sep 1, 2023

@juremedvesek @uscinski Can you please take a look? Thanks in advance.

@augustinosusky
Copy link
Contributor Author

Hello, @juremedvesek @uscinski ,
please, let me know in case you have additional comments or requests. I'll be happy to address them.
Best Regards,
Augustin

Copy link
Contributor

@juremedvesek juremedvesek left a comment

Choose a reason for hiding this comment

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

LGTM. But would like to ask, what is additional value - to be able to use different API's inside url module, or to cover installation, that do not expose service now on root? Thanks.

@juremedvesek
Copy link
Contributor

@Akasurde - changes looks good, they make additional extensions possible.

@augustinosusky
Copy link
Contributor Author

LGTM. But would like to ask, what is additional value - to be able to use different API's inside url module, or to cover installation, that do not expose service now on root? Thanks.

Hello @juremedvesek,
the main reason for adding these is to allow usage of API gateways which serve as proxies to multiple services' API endpoints, SNOW among them. We have such an API GW and:

  • it uses part of the url to forward the request to SNOW API. In our case we access "https://<API_GW_HOST>/servicenow/v1/now" and it gets forwarded to "https://<SNOW_HOST>/api/now"
    • this is controlled by "api_path" parameter
  • it needs own authentication token in headers to authenticate against API GW as well
    • this is controlled indirectly by allowing "custom_headers" parameter as every API GW will probably have own name for the token

@augustinosusky
Copy link
Contributor Author

Hello @juremedvesek,
please, let me know if my last comment is sufficient or you would like to address any other topics.
Cheers,
Augustin

@juremedvesek
Copy link
Contributor

@augustinosusky can you please ping @Akasurde . From my point it is ok, but also him must confirm.

@augustinosusky
Copy link
Contributor Author

Hello @Akasurde ,
please let me know if all is ok also from your side.
Thank you!
Augustin

@augustinosusky
Copy link
Contributor Author

Hello @Akasurde , is there anything else needed to be addressed?
Thank you!
Augustin

@Akasurde
Copy link
Member

@augustinosusky Sorry for the delay. I was afk.

@softwarefactory-project-zuul
Copy link

Build failed (gate pipeline). For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

https://ansible.softwarefactory-project.io/zuul/buildset/cc69dcc2d8e549f2b887907fbfad9af0

✔️ build-ansible-collection SUCCESS in 6m 48s
✔️ ansible-test-sanity-docker-devel SUCCESS in 6m 51s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 37s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 14m 33s
✔️ ansible-test-sanity-docker-stable-2.10 SUCCESS in 12m 48s
ansible-test-sanity-docker-stable-2.11 FAILURE in 15m 24s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 7m 12s
✔️ ansible-test-units-servicenow-itsm-python27 SUCCESS in 6m 13s
✔️ ansible-test-units-servicenow-itsm-python36 SUCCESS in 6m 05s
✔️ ansible-test-units-servicenow-itsm-python37 SUCCESS in 5m 52s
✔️ ansible-test-units-servicenow-itsm-python38 SUCCESS in 4m 30s
✔️ ansible-galaxy-importer SUCCESS in 3m 56s

@augustinosusky
Copy link
Contributor Author

@Akasurde
No problem at all, thank you for accepting the pull request.
Unfortunately, there was again some internal api call timeout in the build process and one of the checks timed out on http request read. Not sure why the automation gets so many timeouts in last months.

@Akasurde Akasurde added mergeit and removed mergeit labels Oct 23, 2023
@softwarefactory-project-zuul
Copy link

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/b98707efdeec45dbb4911c0d71143b42

✔️ build-ansible-collection SUCCESS in 6m 40s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 51s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 8m 57s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 15m 28s
✔️ ansible-test-sanity-docker-stable-2.10 SUCCESS in 9m 20s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 14m 45s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 30s
✔️ ansible-test-units-servicenow-itsm-python27 SUCCESS in 6m 23s
✔️ ansible-test-units-servicenow-itsm-python36 SUCCESS in 6m 20s
✔️ ansible-test-units-servicenow-itsm-python37 SUCCESS in 5m 51s
✔️ ansible-test-units-servicenow-itsm-python38 SUCCESS in 6m 01s
✔️ ansible-galaxy-importer SUCCESS in 3m 19s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 142e374 into ansible-collections:main Oct 23, 2023
1 check passed
@Akasurde
Copy link
Member

@augustinosusky Thanks for the contribution. @juremedvesek Thanks for the review.

@augustinosusky
Copy link
Contributor Author

@augustinosusky , @juremedvesek thank you very much for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants