Add dockerised setup for iGrant.io Consent BB solution

[georgepadayatti]

Description

This PR contains the setup for test environment of iGrant.io Consent Building Block solution.

Related Issue

As discussed in Consent BB WG weekly call on 2023-12-01 and mentioned here.

Motivation and Context

As an initial setup towards running test harness against solution.

[benjaoming]

@georgepadayatti this is great!

Have you had any time to look at the caddy implementation here? https://github.com/GovStackWorkingGroup/bb-consent/blob/main/examples/mock/Dockerfile_caddy

It will use the API spec such that all HTTP requests & responses are validated against the API spec.

[benjaoming]

@georgepadayatti GitHub displays it as if examples/igrantio/consent-openapi.yaml is copied: Please symlink the one in ../../api/consent-openapi.yaml so we don't have multiple copies. Apologies if this is an issue with GitHub.

[benjaoming]

@dborowiecki @pgesek @conradsp Notice on Circle CI: The test runner doesn't perform any tests, however the Circle CI configuration is actually running fine, as it allows PRs. I don't remember what has been the previous conversation around this, but I would like PRs from forked repositories like this one to work. However, I think that there is a security hindrance somewhere preventing these third-party PRs from running.

IMO, it should be possible for any third-party vendor to contribute a new solution to a BB repository without having push access to the main repo.

If you agree, I can open a Jira issue.

[georgepadayatti]

@georgepadayatti this is great!

Have you had any time to look at the caddy implementation here? https://github.com/GovStackWorkingGroup/bb-consent/blob/main/examples/mock/Dockerfile_caddy

It will use the API spec such that all HTTP requests & responses are validated against the API spec.

Sorry for the delayed response. I have looked into your custom caddy setup for OpenAPI validation and updated ours.

Some observations:

1. In your example, the Caddyfile mentioned here is not configured correctly. Due to this OpenAPI validation never occurs. The reverse proxy should be configured outside the route block. Providing ours for reference - link.

2. Now once configured correctly, for automated OpenAPI validation to work - security configuration should be removed from OpenAPI spec.

3. Noticed some discrepancies in the OpenAPI spec. For example id and version are required fields in objects. This should not be the case as both id and version are auto-assigned by the system. We should remove them from the required field in OpenAPI spec as they are also used in the request body.

bb-consent/api/consent-openapi.yaml

Lines 2174 to 2178 in 3f7d2e2

	required:
	- id
	- name
	- version
	- url

Hope we can rectify them for a smooth validation process.

[georgepadayatti]

@georgepadayatti GitHub displays it as if examples/igrantio/consent-openapi.yaml is copied: Please symlink the one in ../../api/consent-openapi.yaml so we don't have multiple copies. Apologies if this is an issue with GitHub.

I hope point 2 in this response clarifies the reason. This is a temporary measure I have taken.

[benjaoming]

This is moving along well, but we need to figure out how to get it running in Circle CI. I'll let you know soon if that requires pushing a local branch to this repo instead.

[georgepadayatti]

@georgepadayatti this is great!
Have you had any time to look at the caddy implementation here? https://github.com/GovStackWorkingGroup/bb-consent/blob/main/examples/mock/Dockerfile_caddy
It will use the API spec such that all HTTP requests & responses are validated against the API spec.

Sorry for the delayed response. I have looked into your custom caddy setup for OpenAPI validation and updated ours.

Some observations:

1. In your example, the Caddyfile mentioned here is not configured correctly. Due to this OpenAPI validation never occurs. The reverse proxy should be configured outside the route block. Providing ours for reference - link.
2. Now once configured correctly, for automated OpenAPI validation to work - security configuration should be removed from OpenAPI spec.
3. Noticed some discrepancies in the OpenAPI spec. For example id and version are required fields in objects. This should not be the case as both id and version are auto-assigned by the system. We should remove them from the required field in OpenAPI spec as they are also used in the request body.

bb-consent/api/consent-openapi.yaml

Lines 2174 to 2178 in 3f7d2e2

	required:
	- id
	- name
	- version
	- url

Hope we can rectify them for a smooth validation process.

@benjaoming One more clarification on Point 1. Once configured, the caddy module didn't work out of the box for me as it was always looking for OPA policies.

I had to fork the caddy module remove the OPA check and rebuild caddy FYI. https://github.com/georgepadayatti/caddy-openapi

[benjaoming]

I had to fork the caddy module remove the OPA check and rebuild caddy FYI.

What does this check do exactly?

[georgepadayatti]

I had to fork the caddy module remove the OPA check and rebuild caddy FYI.

What does this check do exactly?

Remove the checks for Open Policy Agent. This feature is not required for OpenAPI validation. Also it breaks the caddy module.

[benjaoming]

@georgepadayatti do you think that this can be suggested as a PR for the upstream project? It would be nice if we can continue relying on it. I wasn't aware that it had broken.

Perhaps a PR that suggests to comment out the code or make it configurable would be accepted and released by the maintainer.

(It has been working, but it's perhaps 1 year ago since I added it! When I was initially researching it, I was able to see valid requests accepted and invalid requests rejected.)

[lruzicki]

I left one comment. I was able to run tests against this application locally after my small change in test-entrypoint.sh but I not all the tests pass. While some tests passed, issues with loading user data and fetching data from tests. I checked Keycloak via panel admin, which seemed fine. However, I couldn't resolve all test failures due to unfamiliarity with your application's configuration.

[georgepadayatti]

@benjaoming Sorry for the late response. I have fixed the outstanding comments. Are we good to merge and publish test results?

[benjaoming]

@benjaoming Sorry for the late response. I have fixed the outstanding comments. Are we good to merge and publish test results?

I can start by pulling in these changes to a branch in this repo. This should trigger the Circle CI setup.

(I've also expressed some concern about this sub-optimal workflow to the tech committee)

[benjaoming]

Ah great, it seems that GitHub uses the Commit ID, so once I've pushed this into the upstream repo branch and opened a PR (I think it wasn't active before I also opened the PR), we can see the results directly in this repo 👍

[benjaoming]

52.14 go: downloading go.opentelemetry.io/otel/exporters/otlp v0.20.1
53.07 caddy imports
53.07   github.com/caddyserver/caddy/v2/modules/standard imports
53.07   github.com/caddyserver/caddy/v2/modules/caddyhttp/standard imports
53.07   github.com/caddyserver/caddy/v2/modules/caddyhttp/tracing imports
53.07   go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc imports
53.07   go.opentelemetry.io/otel/exporters/otlp/internal: cannot find module providing package go.opentelemetry.io/otel/exporters/otlp/internal
53.07 caddy imports
53.07   github.com/caddyserver/caddy/v2/modules/standard imports
53.07   github.com/caddyserver/caddy/v2/modules/caddyhttp/standard imports
53.07   github.com/caddyserver/caddy/v2/modules/caddyhttp/tracing imports
53.07   go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc imports
53.07   go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig imports
53.07   go.opentelemetry.io/otel/exporters/otlp/internal/envconfig: cannot find module providing package go.opentelemetry.io/otel/exporters/otlp/internal/envconfig
53.70 2024/01/30 14:26:46 [FATAL] exit status 1
------
failed to solve: process "/bin/sh -c xcaddy build --with github.com/chukmunnlee/caddy-openapi@v0.7.0" did not complete successfully: exit code: 1

How sad. This seems to indicate that it's possible to delete packages in "Go Get". I'm not sure exactly what to do here, do you have time to look into it @georgepadayatti ?

I can start pondering if it's become too expensive to use this Caddy + caddy-openapi extension, it hasn't exactly been stable. It's very valuable that it can provide us free compliance testing of request/response vs. OpenAPI spec, though.

[georgepadayatti]

52.14 go: downloading go.opentelemetry.io/otel/exporters/otlp v0.20.1
53.07 caddy imports
53.07   github.com/caddyserver/caddy/v2/modules/standard imports
53.07   github.com/caddyserver/caddy/v2/modules/caddyhttp/standard imports
53.07   github.com/caddyserver/caddy/v2/modules/caddyhttp/tracing imports
53.07   go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc imports
53.07   go.opentelemetry.io/otel/exporters/otlp/internal: cannot find module providing package go.opentelemetry.io/otel/exporters/otlp/internal
53.07 caddy imports
53.07   github.com/caddyserver/caddy/v2/modules/standard imports
53.07   github.com/caddyserver/caddy/v2/modules/caddyhttp/standard imports
53.07   github.com/caddyserver/caddy/v2/modules/caddyhttp/tracing imports
53.07   go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc imports
53.07   go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig imports
53.07   go.opentelemetry.io/otel/exporters/otlp/internal/envconfig: cannot find module providing package go.opentelemetry.io/otel/exporters/otlp/internal/envconfig
53.70 2024/01/30 14:26:46 [FATAL] exit status 1
------
failed to solve: process "/bin/sh -c xcaddy build --with github.com/chukmunnlee/caddy-openapi@v0.7.0" did not complete successfully: exit code: 1

How sad. This seems to indicate that it's possible to delete packages in "Go Get". I'm not sure exactly what to do here, do you have time to look into it @georgepadayatti ?

I can start pondering if it's become too expensive to use this Caddy + caddy-openapi extension, it hasn't exactly been stable. It's very valuable that it can provide us free compliance testing of request/response vs. OpenAPI spec, though.

Interesting. Where is this coming, when building docker ?

[benjaoming]

@georgepadayatti it's from the failed Circle CI buid - https://circleci.com/gh/GovStackWorkingGroup/bb-consent/738

[georgepadayatti]

@georgepadayatti it's from the failed Circle CI build - https://circleci.com/gh/GovStackWorkingGroup/bb-consent/738

I have checked the same on my local machine. This is due to an old version of chukmunnlee/caddy-openapi being used in the Dockefile_caddy present in the mock.

bb-consent/examples/mock/Dockerfile_caddy

Lines 2 to 3 in 280a32f

	RUN xcaddy build --with github.com/chukmunnlee/caddy-openapi@v0.7.0

Once updated to 0.9.0 the docker build succeeds.

[benjaoming]

Thank you for fixing this @georgepadayatti 👍

[benjaoming]

@georgepadayatti wups, forgot, but now I've pushed it to the other branch, so we can see how it builds...

[benjaoming]

@georgepadayatti it seems like examples/igrant isn't automatically discovered. Can you try renaming test-entrypoint.sh to test_entrypoint.sh?

[benjaoming]

@georgepadayatti so this is valid and running 🥳 but I would ask for this change before continuing:

Try to run diff examples/igrantio/consent-openapi.yaml api/consent-openapi.yaml | less

As you can see, the diff is quite big but most changes are single quotes to double quotes.

Is it possible for you to

1. revert all changes related to formatting in consent-api.yaml ? (otherwise advise me on what automatic linter I can run on the source consent-api.yaml in case I could auto-lint this through the existing tooling)
2. I suppose there are some changes left, but can you save those in a diff and then instead dynamically generate the API spec (consent-api.yaml) from the original by applying the diff with patch? This way, the patch can fail when it doesn't apply cleanly, and we can catch any subsequent issues in this intermediate span?

Otherwise, this looks great and I congratulate you and the team on the big effort that's succeeding!

[benjaoming]

Thanks!

I can see that it's only security policies remaining in the diff @georgepadayatti - so no need to have a diff for this.

Let's get rid of them in the main repo so we can use a symlink directly... I'll be back :)

[georgepadayatti]

Thanks!

I can see that it's only security policies remaining in the diff @georgepadayatti - so no need to have a diff for this.

Let's get rid of them in the main repo so we can use a symlink directly... I'll be back :)

Sounds perfect.

[benjaoming]

@georgepadayatti you can go ahead and merge in the upstream main branch, then symlink consent-openapi.yaml and then I can do a final verification and merge this 👍

[georgepadayatti]

@georgepadayatti you can go ahead and merge in the upstream main branch, then symlink consent-openapi.yaml and then I can do a final verification and merge this 👍

Updated.

[benjaoming]

Seems the build has run. It'll be exciting now to see if it shows up in the test harness report...

[benjaoming]

Ay, forgot to submit a proper review... but as comments suggest, we've gone over some details, and now everything looks great and ready for further progress ✔️