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

Still authenticated after removing creds_helpers from Finch VM configuration #480

Open
ginglis13 opened this issue Jul 17, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@ginglis13
Copy link
Contributor

Follow up from #462 #462 (comment), non-blocking for releasing the ECR credential helper integration with Finch.

Describe the bug
Unexpected behavior in successfully pulling an image from a private ECR repo after removing ecr-login credential helper from Finch configuration.

I no longer have Finch configured with a credsStore in ~/.finch/finch.yaml, but I am still able to auth, push and pull with Finch. This is because ~/.finch/config.json still contains the "credsStore":"ecr-login" entry between inits/removals of VMs. Finch should set/remove this entry automatically by cross referencing ~/.finch/finch.yaml and ~/.finch/config.json on VM initialization.

Steps to reproduce

  1. Stop and remove any existing Finch VM: finch vm stop && finch vm remove
  2. Configreecr-login credential helper in finch.yaml:
creds_helpers: 
  - ecr-login
  1. finch vm init
  2. Pull some image from a private ECR repository
  3. Stop and remove the Finch VM: finch vm stop && finch vm remove
  4. Remove creds_helpers,ecr-login from finch.yaml
  5. finch vm init
  6. Successuly pull image from private ECR repository

Expected behavior

finch vm init cross references finch.yaml with the credential config in ~/.finch/config.json

Screenshots or logs
n/a

Additional context
n/a

@ginglis13 ginglis13 added the bug Something isn't working label Jul 17, 2023
@ginglis13 ginglis13 changed the title Still authenticated after removing creds_helpers from finch VM configuration Still authenticated after removing creds_helpers from Finch VM configuration Jul 17, 2023
@ollypom
Copy link
Contributor

ollypom commented Oct 25, 2023

I have also recently stumbled across this too. I was slightly confused if the user should be configuring the ~/.finch/config.json file themselves or if Finch should own it for them when a cred_helper is set?

The 2 scenarios I hit:

  1. I had the ecr-login configured as a cred_helper, I then deleted the ~/.finch/config.json (to start from scratch as I had a few other entries in there), did a finch vm stop and a finch vm start and I was expecting the config.json to be recreated, it was not.

  2. I'm also not sure what the first time user experience should be. If a user is setting the cred_helper themselves for the first time, will Finch populate the existing but empty ~/.finch/config.json file? In my testing that is not the case and I had to populate it. So therefore should we document that users need to populate this file?

@haytok
Copy link
Member

haytok commented May 14, 2024

Hi, Team
I will work on this issue and will assign it to me.

@haytok haytok self-assigned this May 14, 2024
haytok added a commit to haytok/finch that referenced this issue Jun 5, 2024
Suppose we have configured the `creds_helpers` in `~/.finch/finch.yaml` as
follows, and subsequently initialized a VM (`finch vm init`).

```
cpus: 6
creds_helpers:
    - ecr-login
memory: 8GiB
vmType: vz
rosetta: true
```

As a result, `~/.finch/config.json` is created, and it contains the
following:

```
{"credsStore":"ecr-login"}
```

This allows us to utilize the Amazon ECR Docker Credential Helper within Finch.

Subsequently, suppose we stop and remove the VM
(`finch vm stop` && `finch vm remove`), and then remove the
`creds_helpers` configuration from `finch.yaml`.

We then configure the `finch.yaml` file as follows:

```
cpus: 6
memory: 8GiB
vmType: vz
rosetta: true
```

As a result, when we reinitialize the VM (`finch vm init`), the expected
behavior is that it will no longer use the Amazon ECR Docker Credential Helper.

However, when initializing the VM, despite the absence of `creds_helpers`
configuration in `finch.yaml`, the `"credsStore": "ecr-login"` remains in
`config.json`, allowing the continued use of the Amazon ECR Docker
Credential Helper.

This behavior has been reported in the following issue:

- runfinch#480

Furthermore, this issue occurs when we stop the VM (`finch vm stop`),
modify `finch.yaml`, and subsequently start the VM (`finch vm start`).

Consequently, we will modify the behavior to update `config.json` in
accordance with the `creds_helpers` configuration in `finch.yaml` when
initiating or starting the VM.

Signed-off-by: Hayato Kiwata <haytok@amazon.co.jp>
haytok added a commit to haytok/finch that referenced this issue Jun 8, 2024
Suppose we have configured the `creds_helpers` in `~/.finch/finch.yaml` as
follows, and subsequently initialized a VM (`finch vm init`).

```
cpus: 6
creds_helpers:
    - ecr-login
memory: 8GiB
vmType: vz
rosetta: true
```

As a result, `~/.finch/config.json` is created, and it contains the
following:

```
{"credsStore":"ecr-login"}
```

This allows us to utilize the Amazon ECR Docker Credential Helper within Finch.

Subsequently, suppose we stop and remove the VM
(`finch vm stop` && `finch vm remove`), and then remove the
`creds_helpers` configuration from `finch.yaml`.

We then configure the `finch.yaml` file as follows:

```
cpus: 6
memory: 8GiB
vmType: vz
rosetta: true
```

As a result, when we reinitialize the VM (`finch vm init`), the expected
behavior is that it will no longer use the Amazon ECR Docker Credential Helper.

However, when initializing the VM, despite the absence of `creds_helpers`
configuration in `finch.yaml`, the `"credsStore": "ecr-login"` remains in
`config.json`, allowing the continued use of the Amazon ECR Docker
Credential Helper.

This behavior has been reported in the following issue:

- runfinch#480

Furthermore, this issue occurs when we stop the VM (`finch vm stop`),
modify `finch.yaml`, and subsequently start the VM (`finch vm start`).

Consequently, we will modify the behavior to update `config.json` in
accordance with the `creds_helpers` configuration in `finch.yaml` when
initiating or starting the VM.

Note that in this pull request, the commits are divided as follows:

- Implement the logic to update `config.json` according to `finch.yaml`
  and change the function name (loadFinchConfig) in the config package
- Modify to call the logic to update `config.json` according to
  `finch.yaml` on the VM side
- Add unit tests for the credhelper package and modify unit tests for the
  config package
- Modify and add Behavior-Driven Development (BDD) Tests using Ginkgo on
  the VM side
- Add e2e tests

On the other hand, in this commit, the logic to update `config.json`
according to `finch.yaml` is implemented and the function name
(loadFinchConfig) is changed in the config package.

Signed-off-by: Hayato Kiwata <haytok@amazon.co.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants