Skip to content
This repository has been archived by the owner on Jun 25, 2024. It is now read-only.

Support for ingesting KSM crypto key rings and crypto keys #31

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

austinkelleher
Copy link
Contributor

NOTE: This branches off of initial-networking

Copy link
Contributor

@ndowmon ndowmon left a comment

Choose a reason for hiding this comment

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

Just a few small notes.

@@ -94,6 +96,7 @@ The following relationships are created/mapped:
| `google_compute_subnetwork` | **HAS** | `google_compute_instance` |
| `google_iam_service_account` | **ASSIGNED** | `google_iam_role` |
| `google_iam_service_account` | **HAS** | `google_iam_service_account_key` |
| `google_kms_key_ring` | **HAS** | `google_kms_crypto_key` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any relationship that should be created between a higher-level account or service and google_kms_key_ring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no relationship between service account and crypto key/ring. Service account keys are different. We have a google_iam_service_account_key for that. We should consider creating the relationship with the account, but I'll tackle this separately.

@@ -0,0 +1,10 @@
export const STEP_CLOUD_KMS_KEY_RINGS = 'fetch-kms-key-rings';
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question as I posed in your network resources PR - just wondering if it'd be nice to start moving in the direction of

const Entities = {
  KMS_KEY_RING: {
    resourceName: 'KMS Key Ring',
    _type: 'google_kms_key_ring',
    _class: 'Vault',
  },
  KMS_CRYPTO_KEY: {
    resourceName: 'KMS Crypto Key',
    _type: 'google_kms_crypto_key',
    _class: ['Key', 'CryptoKey'],
  },
}

Your call. Obviously this would some inconsistencies with the rest of the services you've already implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I plan to do this in bulk separately.

Copy link
Contributor

@ndowmon ndowmon left a comment

Choose a reason for hiding this comment

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

Thanks for responding to my comments. Looks good.

@austinkelleher austinkelleher merged commit ef48aa0 into initial-networking Oct 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants