ansible-collections / community.hashi_vault

Ansible collection for managing and working with HashiCorp Vault.
https://docs.ansible.com/ansible/devel/collections/community/hashi_vault/index.html
GNU General Public License v3.0
81 stars 62 forks source link

hashi_vault collection - add support for python requirements discovery in `ansible-builder` #105

Closed erinn closed 3 years ago

erinn commented 3 years ago
SUMMARY

Allows ansible-builder to automatically discover python library requirements for execution environments.

ISSUE TYPE
COMPONENT NAME

None

ADDITIONAL INFORMATION

ansible-builder documentation for collections.. This work should not have any effect on anything else, but simply makes it easier to build execution environments for ansible tower/awx.

briantist commented 3 years ago

Hello again @erinn ! I'm going to take a closer look into builder and other collections that publish a requirements.txt in the root, just to ensure I'm not missing any details, but it seems like a good idea. We'll likely want to reuse tests/integration/requirements.txt which properly limits the compatible versions of hvac based on python version.

Additionally, I need to update that as well re: hvac dropping support for python 3.5 in v1.0.0 (as announced just yesterday), but this PR doesn't need to wait for that.

I'd also like to see a changelog fragment for this, so that other builder/AWX users such as yourself will see it in release notes.

Thank you!

codecov[bot] commented 3 years ago

Codecov Report

Merging #105 (1ce020f) into main (98dd60c) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #105   +/-   ##
=======================================
  Coverage   83.52%   83.52%           
=======================================
  Files          16       16           
  Lines         892      892           
  Branches       87       87           
=======================================
  Hits          745      745           
  Misses        129      129           
  Partials       18       18           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 98dd60c...1ce020f. Read the comment docs.

erinn commented 3 years ago

azure.azcollections is one example that uses the meta/execution-environments.yml file, vmware_rest is another and they use requirements.txt just for reference. I'll add a fragment.

I can't of course say for sure what is correct, but it seems to be working for some other folks.

briantist commented 3 years ago

0001-prepare-builder-requirements-CI-changelog.patch.txt

Hey @erinn thanks for your patience, I've looked into this a little bit, and I've made some changes. I was not able to push them into your branch (you may not have ticked the box to allow maintainers to make changes when you created the PR), so I'm including a .patch file you can use to apply the changes to your branch and push it. Let me know what you think.


Summary


attachment contents:

From 0db74699dd0c2704ad177b7ff9c9a84608519556 Mon Sep 17 00:00:00 2001
From: Brian Scholer <1260690+briantist@users.noreply.github.com>
Date: Sun, 11 Jul 2021 13:39:30 -0400
Subject: [PATCH] prepare builder requirements, CI, changelog

---
 .github/workflows/ansible-builder.yml         | 40 +++++++++++++++++++
 .../fragments/105-support-ansible-builder.yml |  3 ++
 meta/ee-requirements.txt                      |  4 ++
 meta/execution-environment.yml                |  4 ++
 requirements.txt                              |  1 -
 5 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 .github/workflows/ansible-builder.yml
 create mode 100644 changelogs/fragments/105-support-ansible-builder.yml
 create mode 100644 meta/ee-requirements.txt
 create mode 100644 meta/execution-environment.yml
 delete mode 100644 requirements.txt

diff --git a/.github/workflows/ansible-builder.yml b/.github/workflows/ansible-builder.yml
new file mode 100644
index 0000000..5213ecf
--- /dev/null
+++ b/.github/workflows/ansible-builder.yml
@@ -0,0 +1,40 @@
+---
+name: ansible-builder
+on:
+  push:
+    paths:
+      - 'meta/execution-environment.yml'
+      - 'meta/ee-requirements.txt'
+  pull_request:
+    paths:
+      - 'meta/execution-environment.yml'
+      - 'meta/ee-requirements.txt'
+  schedule:
+    - cron: '0 13 * * *'
+
+env:
+  NAMESPACE: community
+  COLLECTION_NAME: hashi_vault
+
+jobs:
+  builder:
+    name: ansible-builder requirements
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v2
+        with:
+          path: ansible_collections/${{ env.NAMESPACE }}/${{ env.COLLECTION_NAME }}
+
+      - name: Set up Python
+        uses: actions/setup-python@v2
+        with:
+          python-version: 3.9
+
+      - name: Install ansible-builder
+        run: pip install ansible-builder
+
+      # this is kind of a naive check, since we aren't comparing the output to anything to verify
+      # so the only we'll catch with this is an egregious error that causes builder to exit nonzero
+      - name: Verify Requirements
+        run: ansible-builder introspect --sanitize .
diff --git a/changelogs/fragments/105-support-ansible-builder.yml b/changelogs/fragments/105-support-ansible-builder.yml
new file mode 100644
index 0000000..431e4c8
--- /dev/null
+++ b/changelogs/fragments/105-support-ansible-builder.yml
@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - hashi_vault collection - add ``execution-environment.yml`` and a python requirements file to better support ``ansible-builder`` (https://github.com/ansible-collections/community.hashi_vault/pull/105).
diff --git a/meta/ee-requirements.txt b/meta/ee-requirements.txt
new file mode 100644
index 0000000..a9d32a7
--- /dev/null
+++ b/meta/ee-requirements.txt
@@ -0,0 +1,4 @@
+# ansible-builder doesn't seem to properly handle "; python_version" type of constraints
+# requirements here are assuming python 3.6 or higher
+hvac >=0.10.6
+urllib3 >= 1.15
diff --git a/meta/execution-environment.yml b/meta/execution-environment.yml
new file mode 100644
index 0000000..c899493
--- /dev/null
+++ b/meta/execution-environment.yml
@@ -0,0 +1,4 @@
+---
+version: 1
+dependencies:
+  python: meta/ee-requirements.txt
diff --git a/requirements.txt b/requirements.txt
deleted file mode 100644
index 3e6a595..0000000
--- a/requirements.txt
+++ /dev/null
@@ -1 +0,0 @@
-hvac
-- 
2.31.1
erinn commented 3 years ago

Hmm interesting, the option is checked to allow edits, so I am not sure what is going on there.

What you've got looks good, I can pull it in or we can try to figure out this 'Allow edits by maintainers' thing.

briantist commented 3 years ago

I most likely did something wrong then 😅 but yeah go ahead and pull it in whichever way is easiest. Thanks!

briantist commented 3 years ago

heh confirmed it was me who did not push correctly 🤦 just added a small change to add in the AWS requirements after some feedback in IRC (thanks @tadeboro )

briantist commented 3 years ago

@erinn thanks for bringing this to my attention, I'll be looking to ensure the collection is EE-ready and supported, please don't hesitate to raise any issues you see with that.

I have some other changes I'm planning to wrap up soon that will be released with this change in 1.3.2, so I hope to maybe release that by this weekend or so.