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 init process #367

Merged
merged 4 commits into from
Nov 8, 2022
Merged

Use init process #367

merged 4 commits into from
Nov 8, 2022

Conversation

squaremo
Copy link
Contributor

@squaremo squaremo commented Nov 4, 2022

The operator runs as PID 1, which is expected to reap zombie processes; since it doesn't, they get left around to take up room. This PR installs tini and uses it as PID 1, instead.

I've removed the build/bin/{entrypoint,user_setup} scripts, which aren't necessary. The entry point can be given in the Dockerfile, and useradd already does the necessary setup. It is still necessary to create $HOME/.ssh, since git uses SSH and SSH expects that directory to exist.

This also removes the extra ceremony around using ssh-agent. Very little explanation is given in #92 where it was added, so I did some investigation. It turns out that go-git will use SSH agent if not given any other auth. The automation API will supply auth to go-git as long as it's given something, but will otherwise let go-git fall back to using the ssh-agent.

The problem with falling back to ssh-agent is that it won't work inside the operator container -- even if you run ssh-agent, that will avoid go-git complaining about not finding its socket (which seems to have been the impetus for #92), but will then fail to authenticate because ssh-agent doesn't have any keys to offer. A better alternative is to explicitly require a secret key in .spec.gitAuth.

@squaremo squaremo force-pushed the use-init-process branch 3 times, most recently from e1f9024 to 35f3065 Compare November 8, 2022 09:34
@squaremo squaremo requested a review from lblackstone November 8, 2022 09:34
The operator runs as PID 1, which is expected to reap zombie processes;
since it doesn't, they get left around to take up room. This commit
installs `tini` and uses it as PID 1, instead.

This removes the use of ssh-agent in the `build/bin/entrypoint` script,
which serves only to prevent go-git from complaining about not finding
an ssh-agent socket -- and doesn't help with authentication (if no SSH
key is supplied, it will simply fail the SSH handshake).

Signed-off-by: Michael Bridgen <[email protected]>
These are relics of old operator-sdk boilerplate. In particular,

 - the entrypoint script is not needed because the entrypoint can be
   given in the Dockerfile (and doesn't need to do anything fancy)

 - the user_setup script isn't needed when `useradd` is run. `useradd`
   would not normally be available, since the base image used for
   controllers is often some variation of minimal, distro-less image;
   but, this image uses the maximalist pulumi/pulumi base image.

Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
When using SSH, a key must be obtained from somewhere. On the command
line, git would either use the ssh-agent socket, or try to use a key in
~/.ssh. go-git mirrors this, by resorting to ssh-agent if it is not
given any other choices. But in the operator container, it doesn't make
sense too try to use ssh-agent, because there's no chance to add keys to
it -- its only purpose would be to stop go-git from complaining.

So: treat it as an error if someone uses an SSH git URL, but doesn't
supply a private SSH key.

Signed-off-by: Michael Bridgen <[email protected]>
@squaremo squaremo merged commit 36e3249 into master Nov 8, 2022
@squaremo squaremo deleted the use-init-process branch November 8, 2022 17:20
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