-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add missing fields to compute resources [KMS and disks] #12672
base: main
Are you sure you want to change the base?
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @NickElliot, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
@NickElliot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_instance" "primary" {
attached_disk {
disk_encryption_service_account = # value needed
}
boot_disk {
device_name = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
initialize_params {
architecture = # value needed
enable_confidential_compute = # value needed
source_image_encryption_key {
kms_key_service_account = # value needed
}
source_snapshot_encryption_key {
kms_key_service_account = # value needed
}
}
}
}
Resource: resource "google_compute_instance_from_machine_image" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
image = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
}
kms_key_self_link = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
Resource: resource "google_compute_instance_from_template" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
auto_delete = # value needed
device_name = # value needed
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
enable_confidential_compute = # value needed
image = # value needed
labels = # value needed
provisioned_iops = # value needed
provisioned_throughput = # value needed
resource_manager_tags = # value needed
resource_policies = # value needed
size = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
storage_pool = # value needed
type = # value needed
}
kms_key_self_link = # value needed
mode = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
Resource: resource "google_compute_instance_template" "primary" {
disk {
architecture = # value needed
}
}
Resource: resource "google_compute_region_instance_template" "primary" {
disk {
architecture = # value needed
disk_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
}
|
Tests analyticsTotal tests: 4399 Click here to see the affected service packages
Action takenFound 3931 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
Need to remove |
@GoogleCloudPlatform/terraform-team @NickElliot This PR has been waiting for review for 1 week. Please take a look! Use the label |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for delay with review -- can you resolve the conflict so I cna run tests again? thanks!
a93ec64
to
6e799aa
Compare
Resolved conflicts |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_instance" "primary" {
attached_disk {
disk_encryption_service_account = # value needed
}
boot_disk {
device_name = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
initialize_params {
architecture = # value needed
enable_confidential_compute = # value needed
source_image_encryption_key {
kms_key_service_account = # value needed
}
source_snapshot_encryption_key {
kms_key_service_account = # value needed
}
}
}
}
Resource: resource "google_compute_instance_from_machine_image" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
}
kms_key_self_link = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
Resource: resource "google_compute_instance_from_template" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
auto_delete = # value needed
device_name = # value needed
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
enable_confidential_compute = # value needed
image = # value needed
labels = # value needed
provisioned_iops = # value needed
provisioned_throughput = # value needed
resource_manager_tags = # value needed
resource_policies = # value needed
size = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
storage_pool = # value needed
type = # value needed
}
kms_key_self_link = # value needed
mode = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
Resource: resource "google_compute_instance_template" "primary" {
disk {
architecture = # value needed
}
}
Resource: resource "google_compute_region_instance_template" "primary" {
disk {
architecture = # value needed
disk_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
}
|
Tests analyticsTotal tests: 4436 Click here to see the affected service packages
Action takenFound 3970 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
I'm seeing the internal validate fail on Probably will add the conflicts validation in an expander function |
@NickElliot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
@GoogleCloudPlatform/terraform-team @NickElliot This PR has been waiting for review for 1 week. Please take a look! Use the label |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_instance" "primary" {
attached_disk {
disk_encryption_service_account = # value needed
}
boot_disk {
device_name = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
initialize_params {
architecture = # value needed
enable_confidential_compute = # value needed
source_image_encryption_key {
kms_key_service_account = # value needed
}
source_snapshot_encryption_key {
kms_key_service_account = # value needed
}
}
}
}
Resource: resource "google_compute_instance_from_machine_image" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
}
kms_key_self_link = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
Resource: resource "google_compute_instance_from_template" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
auto_delete = # value needed
device_name = # value needed
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
enable_confidential_compute = # value needed
image = # value needed
labels = # value needed
provisioned_iops = # value needed
provisioned_throughput = # value needed
resource_manager_tags = # value needed
resource_policies = # value needed
size = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
storage_pool = # value needed
type = # value needed
}
kms_key_self_link = # value needed
mode = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
Resource: resource "google_compute_instance_template" "primary" {
disk {
architecture = # value needed
}
}
Resource: resource "google_compute_region_instance_template" "primary" {
disk {
architecture = # value needed
disk_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
}
|
Tests analyticsTotal tests: 3654 Click here to see the affected service packages
🔴 Tests were added that are skipped in VCR:
Action takenFound 7 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🟢 Tests passed during RECORDING mode: 🔴 Tests failed when rerunning REPLAYING mode: Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
Could the failing ones be affected by the PR? I've looked through the configs and none of them contain any resources changed in this PR I'll write some tests to fill the gaps pointed by the modular-magician |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats here LGTM, will pass over again when the missing tests are done!
Do we want separate tests for these resources? This will be a copy-paste from the normal instance as they use the same functions
Other than that everything should be covered. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_instance" "primary" {
attached_disk {
disk_encryption_service_account = # value needed
}
boot_disk {
initialize_params {
enable_confidential_compute = # value needed
}
}
}
Resource: resource "google_compute_instance_from_machine_image" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
}
kms_key_self_link = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
Resource: resource "google_compute_instance_from_template" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
auto_delete = # value needed
device_name = # value needed
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
enable_confidential_compute = # value needed
image = # value needed
labels = # value needed
provisioned_iops = # value needed
provisioned_throughput = # value needed
resource_manager_tags = # value needed
resource_policies = # value needed
size = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
storage_pool = # value needed
type = # value needed
}
kms_key_self_link = # value needed
mode = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
|
Tests analyticsTotal tests: 3889 Click here to see the affected service packages
🔴 Tests were added that are skipped in VCR:
Action takenFound 7 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🟢 Tests passed during RECORDING mode: 🔴 Tests failed when rerunning REPLAYING mode: Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
aa584e7
to
fb98245
Compare
This should be detected as tested now. Fixed a bug with reading service accounts. They are kept in fingerprint rather than their fields in the API so changed it to read local values. reneweing the previous question as well
|
@NickElliot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_instance" "primary" {
boot_disk {
initialize_params {
enable_confidential_compute = # value needed
}
}
}
Resource: resource "google_compute_instance_from_machine_image" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
}
kms_key_self_link = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
Resource: resource "google_compute_instance_from_template" "primary" {
attached_disk {
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
kms_key_self_link = # value needed
}
boot_disk {
auto_delete = # value needed
device_name = # value needed
disk_encryption_key_raw = # value needed
disk_encryption_key_rsa = # value needed
disk_encryption_service_account = # value needed
guest_os_features = # value needed
initialize_params {
architecture = # value needed
enable_confidential_compute = # value needed
image = # value needed
labels = # value needed
provisioned_iops = # value needed
provisioned_throughput = # value needed
resource_manager_tags = # value needed
resource_policies = # value needed
size = # value needed
snapshot = # value needed
source_image_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
source_snapshot_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
raw_key = # value needed
rsa_encrypted_key = # value needed
}
storage_pool = # value needed
type = # value needed
}
kms_key_self_link = # value needed
mode = # value needed
}
instance_encryption_key {
kms_key_self_link = # value needed
kms_key_service_account = # value needed
}
}
|
Tests analyticsTotal tests: 3881 Click here to see the affected service packages
🔴 Tests were added that are skipped in VCR:
Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
closes hashicorp/terraform-provider-google#18050
Hi this is a PR to fix feature gap between the API and the provider on disks and encryption. I'll look for any tickets that this fixes and update them later.
Tested this for basic breaking changes (created an instance on upstream provider and did a terraform apply on the custom provider) - no changes in configuration shown
On some of these fields i wasn't able to add a
ConflictsWith
param. LMK if the API errors are sufficient in these cases or do we want this to throw an error on terraform's sideAlso for the
guest_os_features
field i've made it so that it is computed but it will show null until the user sets it in his config. This field changes based on the source used for the disk and is ForceNew, so i think that this it's a safer behavior to have it null until the user specifficaly sets it in his config.Fields added:
google_compute_instance
google_compute_image
google_compute_snapshot
google_compute_instance_template
google_compute_region_instance_template
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.