Adding svn validations basic flow #1

gopidesupavan · 2024-12-01T03:39:30Z

No description provided.

gopidesupavan · 2024-12-01T03:54:14Z

The core driver for action is publish-config file. based on the action rules and mode it operates.

gopidesupavan · 2024-12-01T03:55:01Z

README.md

+The `publish-config.yml` file has composed of multiple rules to validate the artifacts and publish them to PyPI. The configuration file is structured as follows:
+
+```yaml


This config file required to be configured in repos.

gopidesupavan · 2024-12-01T03:55:59Z

README.md

+publishers:
+    name: providers
+    url: https://dist.apache.org/repos/dist/dev/airflow/


Plan is to checkout the project and use the path filed to workout validations

src/scripts/checksum_check.sh

gopidesupavan · 2024-12-01T09:51:16Z

cc: @potiuk

gopidesupavan · 2024-12-01T11:14:08Z

Here some workflows that triggered using this branch:

https://github.com/gopidesupavan/example-workspace/blob/main/publish-config.yml
https://github.com/gopidesupavan/example-workspace/actions/runs/12104340868/job/33747628251

Not sure that publish config structure make sense or not. if there is some better structure i can update :)

potiuk · 2024-12-01T13:04:37Z

action.yml

+        NAME: ${{ fromJSON(steps.config-parser.outputs.pub_config).publishers.rules.signature-check.name }}
+        SCRIPT: ${{ fromJSON(steps.config-parser.outputs.pub_config).publishers.rules.signature-check.script }}
+        KEYS: ${{ fromJSON(steps.config-parser.outputs.pub_config).publishers.rules.signature-check.keys }}
+        KEYS_FILE_LOCATION: ${{ github.workspace }}/${{ inputs.temp-dir }}/KEYS


We can download KEYS from SVN

Ah ... I see we do :) .

potiuk · 2024-12-01T13:10:12Z

action.yml

+      run: |
+        echo "Verifying $NAME"
+        chmod +x $SCRIPT
+        $SCRIPT "$type"


I am not sure if I like this approach whith $SCRIPT. I think much better (and more composable) approach would be to have separate - very small actions for each of those scripts and compose this action out of smaller composable ones. This way we will not have to configure "scripts" in the configuration, but we would configure particular "actions" to be enabled. While it's less flexible, it would be a better abstraction IMHO. I somehow feel being able to provide a "script" to run is almost the same as writing your own action, where what we want, is to provide a user a set of predefined "actions" they can use. We do not want them to write new scripts.

Ah, I see what you mean. That makes sense—using small, composable actions. So, I can split these steps into multiple actions within the same repository. What do you think?

As for the scripts I initially had in mind: the default script we provide would validate the rule. If someone wants to override it and use their custom script, they can specify the script location as a parameter in the config file. For example, in this workflow, the script is sourced from the workflow repository itself https://github.com/gopidesupavan/example-workspace/blob/main/publish-config.yml#L26.

On the other hand, for svn-check, no script is explicitly defined in the workflow https://github.com/gopidesupavan/example-workspace/blob/main/publish-config.yml#L18, so it falls back to the default script available in the action repository: https://github.com/gopidesupavan/gh-svn-pypi-publisher/blob/basic-feat/src/scripts/svn_checker.py.

Does this behavior okay to have?

Ah, I see what you mean. That makes sense—using small, composable actions. So, I can split these steps into multiple actions within the same repository. What do you think?

Yep.

As for the scripts I initially had in mind: the default script we provide would validate the rule. If someone wants to override it and use their custom script, they can specify the script location as a parameter in the config file. For example, in this workflow, the script is sourced from the workflow repository itself https://github.com/gopidesupavan/example-workspace/blob/main/publish-config.yml#L26.

I see, but if we we have small composable sub-actions, then it's even less effort to replace such "sub-action" from "our" repo, with your own "sub-action" you write - following the same pattern and writing your own hard-coded script. So in this case the users will get the "customizability" at the "sub-action" level, not the "script" level. Which I think is much better.

Say (conceptually):

Our "composed" action:

Action "Publish to PyPI" Sub action read configuration Sub action verify SVN sub action verify Cheksum Sub action verify Signature Sub action Push To PyPI

And someone who would like to customize it, would copy our action to their repo and do this

Action "Publish to PyPI (my own)" Sub action read configuration Sub action verify SVN (my own) sub action verify Cheksum Sub action verify Signature Sub action Push To PyPI

Ideally the "composed" action will be very simple and easy to copy and replace parts of it.

Yes small composable actions will definitely help here better way to handle these subsections and lot more easier for me to add this aswell :) great suggestion small composable action 🚀

potiuk · 2024-12-01T13:13:37Z

publish-config.yml

+      - .tar.gz
+      - .tar.gz.asc
+      - .tar.gz.sha512
+      - -py3-none-any.whl


We Likely need to make it a bit more flexible and allow regexp here. While Airlow only publishes one wheel, many of other projects that have some binary components will have multiple wheel files for many architectures/ ABIs - https://peps.python.org/pep-0491/#file-name-convention

This is not hardcoded one, any file extension can be provided from the config file. Sure, agree can update with regex support. :)

potiuk · 2024-12-01T13:14:16Z

src/scripts/checksum_check.sh

+
+for i in *.asc
+do
+   echo -e "Checking $i\n"; gpg --verify $i


Nice to have... add colors.

potiuk · 2024-12-01T13:20:03Z

src/scripts/checksum_check.sh

@@ -0,0 +1,15 @@
+#!/bin/bash


Also... One experience from the past... While it is seemingly easier to write BASH scripts in simple cases, it's VERY quickly when the scripts become too complex/difficult to understand and maintain. I'd rather rewrite all those scripts in Python and use uv to run them in github actions with the nice support for standalone scripts that uv already has:

https://docs.astral.sh/uv/guides/scripts/#declaring-script-dependencies

This is PEP723 - https://peps.python.org/pep-0723/ that uv already supports and sooner or later it will be natively supported in Python environment (because that PEP is already approved). I think it's the right time to start using it in our automation.

Lack of this "inline dependency declarations" was for me the one reason in the past I prefered to choose bash over Python, but since it is already there, and we have uv support for running such scripts and automatically creating and using the venv that is needed to do it, has removed the last obstacle for me where python scripts were not as good as bash scripts. With this one, I have absolutely no reason to use bash for scripting.

(one thing it allows for example to add colors by declaring rich as dependency in such inline declaration).

Yes sure, I like the python scripts to do these tasks. as this is initial draft and tried existing process in this. :). agree bash is bit complex to understand when it grows to bigger 😄

potiuk · 2024-12-01T13:26:59Z

src/scripts/config_parser.py

@@ -0,0 +1,44 @@
+import os


How about declaring the config structure via json schema (and adding inline requirements to the script to do so).

I think in this case, rather than parsing the file with dedicated parser, we could add validaton in the schema and convert the .yml file into outputs in a generic way:

x: y: z

=> outputs.x.y.z

Sure thats possible we can do that, and thats easy to extract output aswell :) in other steps

Was scratching my head to set some config structure. with the subsection approach we dont need bother about script handling which is really good thing and it simplifies :) how about this config structure?. anyone uses these actions they have to send input config like this to action . Any suggestion :) ?

project: Name: some-project Description: Example project for publishing to PyPI publisher: name: providers url: https://dist.apache.org/repos/dist/release/airflow/ path: "airflow/providers/" checks: svn-check: - id: "extension-check" description: "Extension check" identifiers: - type: "regex" pattern: '.*' - id: "package-name-check" description: "Package name check" identifiers: - type: "regex" pattern: 'apache-airflow-providers-(.*?)-python3-none-any.whl' checksum-check: - id: "checksum-check" description: "SHA512 Check" algorithm: "512" signature-check: - id: "signature-check" description: "Signature Check" method: "gpg" keys: "https://dist.apache.org/repos/dist/release/airflow/KEYS" publish: - id: "publish" description: "Publish to PyPI" exclude: - ".*\\.asc" - ".*\\.sha512"

Believe one more check in svn to validate the number file count per package?
eg: our repo has

The following files should be present (6 files): .tar.gz + .asc + .sha512 (one set of files per provider) -py3-none-any.whl + .asc + .sha512 (one set of files per provider)

Config looks good!

I'd say we should just check if each file has .asc and .sha512 - we do not need to fully verify the number of files, just making sure that all of them are signed and checksumed and that they are right - should be enough, so likely what we have in "checksum" and "signature" check will be enough.

Cool :) will use same config then and use Json to validate schema.

potiuk · 2024-12-01T13:31:22Z

Good. Looks very close to what I thought functionally :) !

I added few suggestions on how to make use of some newer features in Python ecosystem that makes Python scripting far more appealing than bash scripting, also some suggestion on making more generic config validation, which might help with both - validating the config automatically/avoiding typos as well as generic conversion of such configuration to GitHub outputs. Also I think splitting it to more composable actions used from the "main" action rather than passing SCRIPTs is - I think - better (and more easily reusable) abstraction.

gopidesupavan · 2024-12-01T13:54:38Z

Good. Looks very close to what I thought functionally :) !

I added few suggestions on how to make use of some newer features in Python ecosystem that makes Python scripting far more appealing than bash scripting, also some suggestion on making more generic config validation, which might help with both - validating the config automatically/avoiding typos as well as generic conversion of such configuration to GitHub outputs. Also I think splitting it to more composable actions used from the "main" action rather than passing SCRIPTs is - I think - better (and more easily reusable) abstraction.

Yeah, those are really great suggestions and will workout those. 😄

gopidesupavan · 2024-12-07T11:32:27Z

There are few yet to be added, tests and final publish action(this is pretty simple)

Does the changes structure fine?

so now if anyone wants to use this, here is the sample workflow they can configure.

name: GH pypi publish
description: 'Publish to PyPI'

on:
  workflow_dispatch:
    inputs:
      publish-config:
        description: 'Path to the publish config file'
        required: true
        default: 'publish-config.yml'
      temp-dir:
        description: 'Temporary directory to checkout svn repo'
        required: false
        default: 'temp-svn-repo'
      mode:
        description: 'Mode to run the action'
        required: false
        default: 'verify'

jobs:
  gh-action-pypi-publish:
    runs-on: ubuntu-latest
    environment: test
    steps:
     - name: Checkout Code
       uses: actions/checkout@v3

     - name: Setup Python
       uses: actions/setup-python@v4
       with:
         python-version: '3.11'

     - name: "Config parser"
       id: config-parser
       uses: ./read-config
       with:
        publish-config: ${{ inputs.publish-config }}

     - name: "Checkout svn ${{ fromJSON(steps.config-parser.outputs.publisher-url) }}"
       id: "svn-checkout"
       uses: github/gh-svn-pypi-publisher/init@main
       with:
         temp-dir: ${{ inputs.temp-dir }}
         repo-url: ${{ fromJSON(steps.config-parser.outputs.publisher-url) }}
         repo-path: ${{ fromJSON(steps.config-parser.outputs.publisher-path) }}

     - name: "Svn check"
       id: "svn-check"
       uses: github/gh-svn-pypi-publisher/svn@main
       with:
        svn-config: ${{ steps.config-parser.outputs.checks-svn }}
        repo-url: ${{ fromJSON(steps.config-parser.outputs.publisher-url) }}
        repo-path: ${{ fromJSON(steps.config-parser.outputs.publisher-path) }}

     - name: "Checksum check"
       id: "checksum-check"
       uses: github/gh-svn-pypi-publisher/checksum@main
       with:
        checksum-config: ${{ steps.config-parser.outputs.checks-checksum }}
        repo-url: ${{ fromJSON(steps.config-parser.outputs.publisher-url) }}
        repo-path: ${{ fromJSON(steps.config-parser.outputs.publisher-path) }}

     - name: "Signature check"
       id: "signature-check"
       uses: github/gh-svn-pypi-publisher/signature@main
       with:
        signature-config: ${{ steps.config-parser.outputs.checks-signature }}
        repo-url: ${{ fromJSON(steps.config-parser.outputs.publisher-url) }}
        repo-path: ${{ fromJSON(steps.config-parser.outputs.publisher-path) }}

or should we abstract above workflow aswell and then final workflow file something look like this below.

name: GH pypi publish
description: 'Publish to PyPI'

on:
  workflow_dispatch:
    inputs:
      publish-config:
        description: 'Path to the publish config file'
        required: true
        default: 'publish-config.yml'
      temp-dir:
        description: 'Temporary directory to checkout svn repo'
        required: false
        default: 'temp-svn-repo'
      mode:
        description: 'Mode to run the action'
        required: false
        default: 'verify'

jobs:
  gh-action-pypi-publish:
    runs-on: ubuntu-latest
    environment: test
    steps:
     - name: Checkout Code
       uses: actions/checkout@v3

    - name: "Publish packages"
       id: "publish-packages"
       uses: github/gh-svn-pypi-publisher/@main
       with:
        checksum-config: ${{ inputs.publish-config }}
        temp-dir ${{ inputs.temp-dir }}
        mode: ${{ inputs.mode}}

this feels like to much abstraction for me ? WDYT :)

gopidesupavan · 2024-12-07T12:51:04Z

This is my understand for publish.

For rc release we download all the files from https://dist.apache.org/repos/dist/dev/airflow/providers/pypi/ and publish to pypi?
For final release candidates, we can use this SOURCE_DIR="${ASF_DIST_PARENT}/asf-dist/dev/airflow/providers" and filter all the packages from https://dist.apache.org/repos/dist/release/airflow/providers/ and upload to pypi?

gopidesupavan · 2024-12-08T19:08:33Z

Alright publish action also added now :)

gopidesupavan · 2024-12-09T13:32:24Z

Alright this ready for review @potiuk :) when you get a chance..

potiuk · 2024-12-09T20:16:53Z

I will look at it tomorrow :)

gopidesupavan · 2024-12-09T23:25:17Z

I will look at it tomorrow :)

Thank you :)

potiuk · 2024-12-10T07:20:13Z

Also - I am going to do a deep dive into this including reading in detail this one pypa/pip#13048 (review) -> pip is working on adding trusted publishing as we speak and there are some real gems there. Once we get it in a shape we are happy with, we might even ask some of the people there to comment on our action + I will get a few people from the ASF to comment here.

potiuk · 2024-12-10T07:21:17Z

So I recommend you to also take a look there :).

gopidesupavan · 2024-12-10T07:49:19Z

Also - I am going to do a deep dive into this including reading in detail this one pypa/pip#13048 (review) -> pip is working on adding trusted publishing as we speak and there are some real gems there. Once we get it in a shape we are happy with, we might even ask some of the people there to comment on our action + I will get a few people from the ASF to comment here.

Yeah that discussion is helpful, as mentioned webknjaz in comments about environment and approval, thats really nice. i actually tried out in another repo using these custom actions it pauses workflow until approval and then it starts further validations. In our case we are not building any builds in this current workflow, we are downloading from svn repo this is at present state :) , i feel. but agree when start adding reproducibility checks and further build process, webknjaz made good security point there, we will also to make sure the publish runs in separate workflow. i will also follow that conversion closely thanks for sharing otherwise i would have missed that :)

Sure absolutely fine to pull people and review, more feedback never hurts , it always improve things better better :)

gopidesupavan · 2024-12-10T08:00:58Z

Like this it will ask approval status when we use environments + reviewers , we can add branches also in the github environment settings :)

publish/action.yml

potiuk · 2024-12-12T23:23:08Z

BTW. Shall we already attempt to move it to airlfow repo and start configuring / dry running it ? I think - since we do not realy need airflow sources - that one sounds like a good idea to go to a separate reposiory and we should already run and test it there together - and invite Elad and others to further iterate on it ?

gopidesupavan · 2024-12-12T23:27:01Z

BTW. Shall we already attempt to move it to airlfow repo and start configuring / dry running it ? I think - since we do not realy need airflow sources - that one sounds like a good idea to go to a separate reposiory and we should already run and test it there together - and invite Elad and others to further iterate on it ?

Yeah , correct we dont need airflow sources we can configure our existing released versions in config and play around :).

Should i refer this action in our airflow? or should move all these actions to airflow? and configure.

potiuk · 2024-12-12T23:30:55Z

Maybe move. I created a repo for you https://github.com/apache/airflow-publish - in about 5-10 minutes you should be able to add things there - you will have write access as with airflow.

gopidesupavan · 2024-12-12T23:31:47Z

Maybe move. I created a repo for you https://github.com/apache/airflow-publish - in about 5-10 minutes you should be able to add things there - you will have write access as with airflow.

oh thats awesome :)

gopidesupavan · 2024-12-12T23:32:43Z

We can do test on airflow 2.10.4 :) its currently in process

potiuk · 2024-12-12T23:37:56Z

We can do test on airflow 2.10.4 :) its currently in process

That would be awesome if we could try it there - first "dry run" of course.

gopidesupavan · 2024-12-13T00:28:42Z

Update now, https://github.com/apache/airflow-publish/tree/airflow-artifacts-publish. it looks like workflow dispatch should be in default branch main. cant trigger from other branches

gopidesupavan · 2024-12-15T07:23:52Z

Okay closing this now..now that we have separate repository exists.

Thanks @potiuk for all the suggestions 🙂 and really appreciate your support and guidance.

gopidesupavan added 2 commits December 1, 2024 03:41

adding svn validations basic flow

10d2269

update names

f77a114

gopidesupavan force-pushed the basic-feat branch from f225097 to f77a114 Compare December 1, 2024 03:42

remove echo

164a1d1

gopidesupavan mentioned this pull request Dec 1, 2024

Make "Trusted Publishing" works for our PyPI releasing apache/airflow#41937

Open

gopidesupavan commented Dec 1, 2024

View reviewed changes

src/scripts/checksum_check.sh Outdated Show resolved Hide resolved

gopidesupavan added 3 commits December 1, 2024 08:22

update tests

ca41ee7

adding signature check

add8003

fix workspace name

9c8703a

potiuk reviewed Dec 1, 2024

View reviewed changes

adding subsection actions

bb946f5

add unit tests for svn check

0779890

gopidesupavan added 3 commits December 7, 2024 16:26

add unit tests to check-sum

94b2215

add pypi publish and tests

7eea894

update variables

bf17e47

gopidesupavan added 3 commits December 9, 2024 13:20

add tests to signature check

cce1d2f

add python-gnupg module to tests

d180e37

add requests module to tests

45e96b9

gopidesupavan requested a review from potiuk December 9, 2024 13:33

gopidesupavan added 3 commits December 9, 2024 13:38

use pytest-unordered

03e48d6

use pytest-unordered

8025fa5

log updates

6e1ec93

adding doc string and fixing lints

481a1b6

remove fromjson for non json outputs

1edb0d4

gopidesupavan added 2 commits December 10, 2024 18:51

update sample workflow usage as per best practice suggestion

6883637

update sample workflow usage as per best practice suggestion

0259bcf

potiuk reviewed Dec 12, 2024

View reviewed changes

publish/action.yml Outdated Show resolved Hide resolved

potiuk reviewed Dec 12, 2024

View reviewed changes

publish/action.yml Outdated Show resolved Hide resolved

rename action

5af7723

gopidesupavan closed this Dec 15, 2024

		The `publish-config.yml` file has composed of multiple rules to validate the artifacts and publish them to PyPI. The configuration file is structured as follows:

		```yaml

Adding svn validations basic flow #1

Adding svn validations basic flow #1

Conversation

gopidesupavan commented Dec 1, 2024

gopidesupavan commented Dec 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gopidesupavan commented Dec 1, 2024

gopidesupavan commented Dec 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

potiuk Dec 1, 2024 • edited Loading

Choose a reason for hiding this comment

potiuk Dec 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gopidesupavan Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

potiuk commented Dec 1, 2024 • edited Loading

gopidesupavan commented Dec 1, 2024

gopidesupavan commented Dec 7, 2024 • edited Loading

gopidesupavan commented Dec 7, 2024

gopidesupavan commented Dec 8, 2024 • edited Loading

gopidesupavan commented Dec 9, 2024

potiuk commented Dec 9, 2024

gopidesupavan commented Dec 9, 2024

potiuk commented Dec 10, 2024

potiuk commented Dec 10, 2024

gopidesupavan commented Dec 10, 2024 • edited Loading

gopidesupavan commented Dec 10, 2024

potiuk commented Dec 12, 2024 • edited Loading

gopidesupavan commented Dec 12, 2024

potiuk commented Dec 12, 2024

gopidesupavan commented Dec 12, 2024

gopidesupavan commented Dec 12, 2024

potiuk commented Dec 12, 2024

gopidesupavan commented Dec 13, 2024 • edited Loading

gopidesupavan commented Dec 15, 2024

potiuk Dec 1, 2024 •

edited

Loading

potiuk Dec 1, 2024 •

edited

Loading

gopidesupavan Dec 2, 2024 •

edited

Loading

potiuk commented Dec 1, 2024 •

edited

Loading

gopidesupavan commented Dec 7, 2024 •

edited

Loading

gopidesupavan commented Dec 8, 2024 •

edited

Loading

gopidesupavan commented Dec 10, 2024 •

edited

Loading

potiuk commented Dec 12, 2024 •

edited

Loading

gopidesupavan commented Dec 13, 2024 •

edited

Loading