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

Apply fails on duplicate key pair #52

Closed
kenazk opened this issue Jan 18, 2019 · 13 comments
Closed

Apply fails on duplicate key pair #52

kenazk opened this issue Jan 18, 2019 · 13 comments
Assignees
Labels
Area: Content Content includes providers and resources Type: Bug Something isn't working

Comments

@kenazk
Copy link
Contributor

kenazk commented Jan 18, 2019

Describe the bug
When running apply, if keypair has previously been created (either through Lyra or not), apply fails with the following:

2019-01-18T10:01:46.732-0800 [DEBUG] lyra.lyra: 2019-01-18T10:01:46.732-0800 [DEBUG] lyra: Creating KeyPair: desired="&{ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCX363gh/q6DGSL963/LlYcILkYKtEjrq5Ze4gr1BJdY0pqLMIKFt/VMJ5UTyx85N4Chjb/jEQhZzlWGC1SMsXOQ+EnY72fYrpOV0wZ4VraxZAz3WASikEglHJYALTQtsL8RGPxlBhIv0HpgevBkDlHvR+QGFaEQCaUhXCWDtLWYw== nyx-test-keypair-nopassword lyra-test-kp }"
2019-01-18T10:01:47.056-0800 [DEBUG] lyra.lyra: 2019-01-18T10:01:47.056-0800 [DEBUG] lyra: Error creating KeyPair: error="InvalidKeyPair.Duplicate: The keypair 'lyra-test-kp' already exists.
2019-01-18T10:01:47.056-0800 [DEBUG] lyra.lyra: 	status code: 400, request id: 701e0653-efbf-4468-b36b-0ff0d7e9a862"
panic: invocation of Aws::KeyPairHandler create failed: EVAL_GO_FUNCTION_ERROR Go function Create returned error 'InvalidKeyPair.Duplicate: The keypair 'lyra-test-kp' already exists.
	status code: 400, request id: 701e0653-efbf-4468-b36b-0ff0d7e9a862'

To Reproduce
Steps to reproduce the behavior:

  1. Write a workflow like
resource keypair {
    input  => ($tags),
    output => ($key_fingerprint)
  }{
    public_key_material => "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCX363gh/q6DGSL963/LlYcILkYKtEjrq5Ze4gr1BJdY0pqLMIKFt/VMJ5UTyx85N4Chjb/jEQhZzlWGC1SMsXOQ+EnY72fYrpOV0wZ4VraxZAz3WASikEglHJYALTQtsL8RGPxlBhIv0HpgevBkDlHvR+QGFaEQCaUhXCWDtLWYw== nyx-test-keypair-nopassword",
    key_name => "lyra-test-kp"
  }
  1. Run command lyra apply attach --debug
  2. Delete identity.db
  3. Re-run command lyra apply attach --debug
  4. See error

Expected behavior
I expect Lyra to record keypair into identity.db and continue forward with execution given the resource already exists as specified.

Logs
If applicable, add the full terminal output (try adding the --debug flag for more verbosity) to help explain your problem.

Environment (please complete the following information):

  • OS: [e.g. macOS 10.14.1]
  • Version/Build number [e.g. 0.0.1, f61edcc]
  • Cloud target [e.g. AWS, GCP]

Additional context
Add any other context about the problem here.

@kenazk kenazk added the Type: Bug Something isn't working label Jan 18, 2019
@thallgren
Copy link
Contributor

The identity.db is solely for mapping internal activity ID's to their external counterpart. It's not intended as a generic shared storage. An important objective with its design is to prevent that lyra is storing "state". We don't want lyra to maintain a copy of the real world, but rather map to it.

In this particular case, I would consider it the providers responsibility to ensure that the public_key_material is updated correctly (or left alone if it's unchanged). Perhaps what's needed here is for the provider to catch the InvalidKeyPair.Duplicate and handle that as a special case or that the provider somehow keeps track of public keys elsewhere.

@kenazk
Copy link
Contributor Author

kenazk commented Jan 23, 2019

Tagging @markfuller

@markfuller
Copy link
Contributor

In this particular case, I would consider it the providers responsibility to ensure that the public_key_material is updated correctly (or left alone if it's unchanged).

This would mean that the provider is instructed to do a create but in fact does an update (note from Kenaz's debug output that it is the handler's Create that is called in this scenario)

@kenazk see #69. Pending resolution of that issue, you could:

  1. pass an external_id to identify existing resources
  2. never mutate that resource (it is considered to be read only)
  3. nor ever add that resource to the identity.db (so must always pass external_id)

(2 and 3 are as-per discussions to-date on this)

@thallgren
Copy link
Contributor

Perhaps I've misunderstood the problem. Is the case here that the public-key actually is the same as the external_id?

@thallgren thallgren reopened this Jan 24, 2019
@thallgren
Copy link
Contributor

I hate that "Close and comment" button 👎

@markfuller
Copy link
Contributor

The key_name is the external_id.

@thallgren
Copy link
Contributor

@markfuller so am I correct to assume that what happens is that during the second apply, the artefact is found but since there's no way to retrieve the public_key_material, that column is always considered to be changed. This, combined with a missing update in the provider causes Lyra to fall back to a delete/create?

@markfuller
Copy link
Contributor

Kenaz's To Reproduce steps involve applying a manifest (to create the resource), deleting the identity db and then reapplying the manifest (to simulate a scenario where we have an unmanaged resource).

So that is why the create is being invoked. The reason it is failing is that there is a uniqueness constraint in AWS on the key_name (i.e. you can't create two key pairs with the same name).

@thallgren
Copy link
Contributor

"to simulate a scenario where we have an unmanaged resource"

In that case I suggest we close this issue because deleting the identity.db doesn't simulate that at all.

@kenazk
Copy link
Contributor Author

kenazk commented Jan 29, 2019

The main issue is "how does Lyra interact with resources it finds in the real world?" When it's being asked to create something that already exists, should it:

  1. Fail and report duplicate
  2. Continue and store record in identity.db
  3. Continue and try to recreate resource

My take is it should do 2.

@thallgren
Copy link
Contributor

I agree that it should do 2.
1 is wrong because it's not really a duplicate since what Lyra has in it's manifest is a desired state, not an actual existing state. If a desired state already exists, well, then all is good, the desire is fulfilled.
3 is wrong because it will never succeed (unless the resource is removed by someone else, which seems dead wrong).

@scottyw scottyw added the Area: Content Content includes providers and resources label Feb 25, 2019
@scottyw scottyw closed this as completed Feb 25, 2019
@scottyw scottyw reopened this Feb 25, 2019
@scottyw
Copy link
Contributor

scottyw commented Mar 7, 2019

FWIW, the TF AWS bridge provider handles this case, by deleting and recreating the pair every time.

@thallgren
Copy link
Contributor

The "native" AWS provider referenced here no longer exists and testing running apply repeatedly with the following manifest works just fine, so I'm closing this ticket.

returns:
  fingerprint:
    type: String
resource: Aws::Key_pair
value:
  public_key: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCX363gh/q6DGSL963/LlYcILkYKtEjrq5Ze4gr1BJdY0pqLMIKFt/VMJ5UTyx85N4Chjb/jEQhZzlWGC1SMsXOQ+EnY72fYrpOV0wZ4VraxZAz3WASikEglHJYALTQtsL8RGPxlBhIv0HpgevBkDlHvR+QGFaEQCaUhXCWDtLWYw== nyx-test-keypair-nopassword"
  key_name_prefix: lyra-test-kp

[EPIC] Lots o' content automation moved this from To do to Done Jun 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Content Content includes providers and resources Type: Bug Something isn't working
Projects
No open projects
Development

No branches or pull requests

4 participants