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

chore: add CI validation for pull requests #217

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Conversation

jonas-jonas
Copy link
Member

Related Issue or Design Document

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
    If this pull request addresses a security vulnerability,
    I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

Further comments

@jonas-jonas
Copy link
Member Author

@aeneasr this would be my proposal to have CI run on SDK PRs (e.g. updating dependencies, etc.).

Alternatively, we could run one workflow per language/client combination, but that got very bloated and is not very clearly usable, IMO. With 5 clients à 10 languages, that would be 50 jobs per PR :D

Are you okay with the format (e.g. one workflow for each language)? If so, I'll make this ready to use after my vacation.

@jonas-jonas jonas-jonas self-assigned this Jan 2, 2023
@jonas-jonas jonas-jonas changed the title chore: add ci for pr chore: add CI validation for pull requests Jan 2, 2023
@jonas-jonas jonas-jonas requested a review from aeneasr January 2, 2023 09:01
@jonas-jonas jonas-jonas mentioned this pull request Feb 6, 2023
6 tasks
@@ -0,0 +1 @@
v1.1.9
Copy link
Member Author

Choose a reason for hiding this comment

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

These will be overridden by the SDK publish task, but need to be added manually here so the tests of this PR pass.

@jonas-jonas
Copy link
Member Author

Next step would be, to update the release pipeline to use GHA as well.

@jonas-jonas jonas-jonas marked this pull request as ready for review April 26, 2023 15:53
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

A bit difficult to review because there are many changes to the cwd. Should we push another kratos release for the SDK to fix first?

jobs:
test:
runs-on: ubuntu-latest
container: oryd/sdk:v0.0.51
Copy link
Member

Choose a reason for hiding this comment

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

We need to keep this in sync now as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we do. But I do intend to clean up the Circle CI pipeline soon-ish as well.

And ideally, I'd like to see if we can split up all these SDK generation processes into different workflows (per language), that use their own base images. But not sure if there are many benefits to that yet.

@jonas-jonas
Copy link
Member Author

@aeneasr updated the branch. PTAL.

@aeneasr
Copy link
Member

aeneasr commented May 24, 2023

It's failing :(

@jonct
Copy link

jonct commented Jun 15, 2023

we could run one workflow per language/client combination, but that got very bloated and is not very clearly usable, IMO. With 5 clients à 10 languages, that would be 50 jobs per PR :D

Please excuse my barging in. I was just looking into Swift language support (#273), and this might actually block that effort.

Apple's official Swift builds come in Ubuntu but not Debian per se. So I'd begun to surgically retrofit the appropriate libraries into the Dockerfile. Gradually applying an embarrassing series of tricks.

Then I started noticing evidence of similar tricks, duplication, and unexpected side-effects already present — in the stanzas for other languages that others had added before me.

It occurs to me that the project might be, essentially… inside-out?

Apart from the WETness of it all — setting aside the distribution of concerns across many repeating parallel lists. By now this unwieldy, monolithic container has begun to undermine its own ability to test the final product for use in broadly heterogenous environments.

Might it help to leverage multi-stage builds here? Something like the following crude scaffolding, giving each language its own native container environment.

You'd still have one pass for each client SDK, but then all languages (and others, and dialects to be easily added later) would still build/test within a single CI context that can pass or fail and block a release.

As a bonus, I think BuildKit would nicely parallelize the bulk of it?

Anyway: it's a thought that I had while struggling to accommodate Swift and Ubuntu dependencies in the current structure.

ARG ORY_SDK_NAME=client
ARG ORY_SDK_VERSION=v1.1.37

FROM openapitools/openapi-generator-cli AS generator
COPY config contrib openapi repos spec /sdk
WORKDIR /sdk

RUN openapi-generator-cli batch \
	-i spec/${ORY_SDK_NAME}/${ORY_SDK_VERSION}.json \
	-o out \
	config/${ORY_SDK_NAME}/*.yml

########################################
## test natively to each environment
########################################

FROM swift:5.8 AS swift

COPY clients/${ORY_SDK_NAME}/swift /test
COPY --from=generator /sdk/out/swift /test/sdk

WORKDIR /test

RUN swift test | tee output.log

########################################

FROM python:3.11 AS python

COPY clients/${ORY_SDK_NAME}/python /test
COPY --from=generator /sdk/out/python /test/sdk

WORKDIR /test

RUN ./test.py | tee output.log

########################################

FROM node:20 AS typescript

COPY clients/${ORY_SDK_NAME}/typescript /test
COPY --from=generator /sdk/out/typescript /test/sdk

WORKDIR /test

RUN node ./test.ts | tee output.log

########################################
## final roundup and commit
########################################

FROM debian:slim

WORKDIR /sdk

COPY --from=generator /sdk /sdk

COPY --from=swift /test/output.log logs/swift.log
COPY --from=python /test/output.log logs/python.log
COPY --from=typescript /test/output.log logs/typescript.log

COPY scripts/* /usr/local/bin/

RUN final-sanity-check.sh

CMD ['release.sh']

@jonas-jonas
Copy link
Member Author

@jonct thanks for the input! This is very valuable.

Might it help to leverage multi-stage builds here? Something like the following crude scaffolding, giving each language its own native container environment.

That could work. One reason for the current setup is that failing builds can easily be debugged on a local environment (in theory, there are issues with the Dart build and M1 macs right now), as you can just pull the image and rerun with the same inputs. I am guessing that could still be easily achieved with a multi-stage build, though.

I'd have two questions that you may be able to answer:

  1. How would you start a specific language build only? Due to the hierarchical nature of docker images, I imagine that to be quite difficult.
  2. A language build failing should not lead to other language builds failing as well.

I was actually thinking about moving the whole workflow to individual steps as GitHub actions jobs. The individual language builds don't have much in common, and are defined as individual functions in scripts/generate.sh, which could in turn be moved to pipeline steps, which can also be easily parallelized. You can then just use the language's docker container as the image in those steps directly, without the oryd/sdk monolith.
To debug, we can use https://github.com/nektos/act, though that has some drawbacks.

This PR serves as a playground to explore those ideas, but due to time constraints, it has gone a bit stale.

@jonct
Copy link

jonct commented Jun 20, 2023

Hi there! Thanks; great questions.

  1. How would you start a specific language build only?

I don't know exactly how any given builder tool handles dependencies. If they're smart enough to parallelize stages until they're needed by other stages, they might also be smart enough to skip stages that they don't need for a given named target stage.

In which case e.g. docker build --target python might be sufficient.

But overall, you'd have a giant reduction in individual package management. The full build might just become quick enough that it doesn't matter? The burden of caching ends up at the container registry level, and might be more effective that way.

  1. A language build failing should not lead to other language builds failing as well.

Oh. Well, that could open up other options as you've suggested. And as you've noted, you could end up with dozens of CI jobs per commit.

I suppose this is a question of what you're aiming to test here: whether 1) a newly modified API spec breaks one or more platforms and should thus be reviewed by humans prior to official release, or if 2) a given language's sample project just needs to be updated to meet the revised API spec, so that one language's binding is slightly delayed relative to the others.

I'm not sure which is best for your internal view of the project. But I'll register my vote that any "tests" of a given language or platform SDK should be conducted atop a native vanilla baseline, accepted as representative of that language or platform.

Thanks for all that you do! The Ory philosophy is filling a great need here, and my team is grateful for your efforts. 🥳

@jonct
Copy link

jonct commented Jun 20, 2023

I was actually thinking about moving the whole workflow to individual steps as GitHub actions jobs. The individual language builds don't have much in common, and are defined as individual functions in scripts/generate.sh, which could in turn be moved to pipeline steps, which can also be easily parallelized.

Another option, BTW: use the repositories for each language SDK to house its own sample project and testing rigs.

Have this project submit generated SDKs as pull requests to the respective projects.

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

Successfully merging this pull request may close these issues.

3 participants