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

Use multistage build for gateway #1506

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Conversation

psschwei
Copy link
Collaborator

@psschwei psschwei commented Oct 3, 2024

No description provided.

Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>
@Tansito Tansito requested a review from a team October 14, 2024 18:23
Copy link
Member

@Tansito Tansito left a comment

Choose a reason for hiding this comment

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

@akihikokuroda could you take a look to this PR? I would like a double check apart from my side around this change 🙏

Regardless of my only comment I see everything fine but I would like another review from Aki if it's possible.

gateway/Dockerfile Show resolved Hide resolved

# APP image from `scratch` which will be the final image
# and remaining application requirements will be installed
FROM scratch AS APP
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder why this APP image is necessary. What is the difference between copying back the ${MICRO_IMAGE_DIR}/ into the BASE image and the APP image?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The BASE image still has things like a package manager and a shell that the gateway really doesn't need, whereas scratch / APP is empty except for what we explicitly add to it. So that does two things: (a) it reduces the number of components that we have to manage for CVEs, and (b) it reduces the threat vectors for that particular container.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I may be wrong but I read

COPY --from=BASE / ${MICRO_IMAGE_DIR}

copies everything in the BASE to ${MICRO_IMAGE_DIR} directory and

COPY --from=BUILD ${MICRO_IMAGE_DIR}/ .

copies everything in the BASE + additional pieces to the APP
So everything in the BASE is copied to APP. Where am I misunderstanding?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You could be right... I'm just copying from what another team already did and didn't look too closely at what they were doing, I just assumed it worked since it had already gone through review.


# hadolint ignore=DL3013
RUN pip install --upgrade --no-cache-dir pip>=24.2 &&\
pip install --upgrade --no-cache-dir setuptools>=72.1.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the setuptools unnecessary after all or is the right version in ubi9-micro:9.4-15?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was assuming that the scratch image doesn't have setuptools so it shouldn't need to be updated. but maybe it will be?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure. Scan will tell us :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all hail general scan 😆

Copy link
Collaborator

@akihikokuroda akihikokuroda left a comment

Choose a reason for hiding this comment

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

I don't see any major issues.

@psschwei psschwei merged commit 4f77fae into Qiskit:main Oct 16, 2024
10 checks passed
@psschwei psschwei deleted the gateway-dockerfile branch October 16, 2024 17:26
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