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

Config indy generic proxy as a sidecar of buildah task #1815

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sswguo
Copy link

@sswguo sswguo commented Jan 14, 2025

Middleware build pipeline needs to access indy for generic proxy requests (like non-java & non-npm artifacts), and it requires the indy generic proxy deployed together with the builder in the same cluster to handle the proxy http format, which openshift routers don't allow.

In MP+, we deployed this as a standalone service, and in Konflux, the sidecar way maybe a better option, but we hope to start discussing based on this PR, to see which one is better. Or if we need a dedicated task to avoid introducing this to all of the users even there are feature flag.

@sswguo sswguo requested a review from a team as a code owner January 14, 2025 03:26
Copy link
Member

@arewm arewm left a comment

Choose a reason for hiding this comment

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

What are the implications if we add this sidecar to every single buildah task that is created? Will this result in a drastic increase in cluster utilization?

Comment on lines +775 to +779
if [ "$(params.ENABLE_INDY_PROXY)" == "true" ]; then
export QUARKUS_OIDC_CLIENT_CLIENT_ID="$(params.INDY_PROXY_CLIENT_ID)"
export QUARKUS_OIDC_CLIENT_CREDENTIALS_SECRET="$(params.INDY_PROXY_CLIENT_CREDENTIAL)"
/deployment/start-service.sh
fi
Copy link
Member

Choose a reason for hiding this comment

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

If we are to implement a sidecar like this, is there any reason why it needs to be specific to Quarkus/INDY? Would this have value as a generalization?

Copy link
Author

@sswguo sswguo Jan 17, 2025

Choose a reason for hiding this comment

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

The service is implemented upon quarkus and both QUARKUS_OIDC_CLIENT_CLIENT_ID and QUARKUS_OIDC_CLIENT_CREDENTIALS_SECRET are supported in quarkus that we can declare the variable as env, which can be declared in the config file as following:

BTW, the client ID(service account) is used to access Indy.

 quarkus
    oidc-client:
       auth-server-url: ""
       client-id: 
       credentials:
           secret: 

@@ -725,3 +754,26 @@ spec:
requests:
cpu: 100m
memory: 256Mi
sidecars:
- name: indy-generic-proxy
image: quay.io/factory2/indy-generic-proxy-service:latest-prod
Copy link
Member

Choose a reason for hiding this comment

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

What is the provenance of this image?

Copy link
Author

Choose a reason for hiding this comment

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

I believe we need to follow konflux pattern to build the image if we agree the sidecar proposal.

@sswguo
Copy link
Author

sswguo commented Jan 17, 2025

What are the implications if we add this sidecar to every single buildah task that is created? Will this result in a drastic increase in cluster utilization?

Seems there is no feature flag in tekton to control the sidecar, so yeah it will result in downloading the image for every single buildah task . Hm.. do we have cache for the image ? if so, it may not download it every time, and we have flag ENABLE_INDY_PROXY in script part to control if the service will be started or not.

Another way to make this is to deploy it in the cluster as a service, like what we are doing in MP+.

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.

2 participants