PrivateCA PR feedback (#4939) (#3411)

Co-authored-by: Riley Karson <[email protected]>
Signed-off-by: Modular Magician <[email protected]>

Co-authored-by: Riley Karson <[email protected]>

Author: modular-magician

.changelog/4939.txt

@@ -0,0 +1,3 @@
+```release-note:none
+
+```

google-beta/resource_privateca_ca_pool.go

@@ -370,13 +370,13 @@ If this is omitted, then this CaPool will not add restrictions on a certificate'
 								Schema: map[string]*schema.Schema{
 									"allow_subject_alt_names_passthrough": {
 										Type:     schema.TypeBool,
-										Optional: true,
+										Required: true,
 										Description: `If this is set, the SubjectAltNames extension may be copied from a certificate request into the signed certificate.
 Otherwise, the requested SubjectAltNames will be discarded.`,
 									},
 									"allow_subject_passthrough": {
 										Type:     schema.TypeBool,
-										Optional: true,
+										Required: true,
 										Description: `If this is set, the Subject field may be copied from a certificate request into the signed certificate.
 Otherwise, the requested Subject will be discarded.`,
 									},
@@ -1160,14 +1160,14 @@ func expandPrivatecaCaPoolIssuancePolicyIdentityConstraints(v interface{}, d Ter
 	transformedAllowSubjectPassthrough, err := expandPrivatecaCaPoolIssuancePolicyIdentityConstraintsAllowSubjectPassthrough(original["allow_subject_passthrough"], d, config)
 	if err != nil {
 		return nil, err
-	} else if val := reflect.ValueOf(transformedAllowSubjectPassthrough); val.IsValid() && !isEmptyValue(val) {
+	} else {
 		transformed["allowSubjectPassthrough"] = transformedAllowSubjectPassthrough
 	}
 
 	transformedAllowSubjectAltNamesPassthrough, err := expandPrivatecaCaPoolIssuancePolicyIdentityConstraintsAllowSubjectAltNamesPassthrough(original["allow_subject_alt_names_passthrough"], d, config)
 	if err != nil {
 		return nil, err
-	} else if val := reflect.ValueOf(transformedAllowSubjectAltNamesPassthrough); val.IsValid() && !isEmptyValue(val) {
+	} else {
 		transformed["allowSubjectAltNamesPassthrough"] = transformedAllowSubjectAltNamesPassthrough
 	}
 

google-beta/resource_privateca_ca_pool_test.go

@@ -77,10 +77,10 @@ resource "google_privateca_ca_pool" "default" {
     maximum_lifetime = "50000s"
     allowed_issuance_modes {
       allow_csr_based_issuance = true
-      allow_config_based_issuance = true
+      allow_config_based_issuance = false
     }
     identity_constraints {
-      allow_subject_passthrough = true
+      allow_subject_passthrough = false
       allow_subject_alt_names_passthrough = true
       cel_expression {
         expression = "subject_alt_names.all(san, san.type == DNS || san.type == EMAIL )"
@@ -100,7 +100,7 @@ resource "google_privateca_ca_pool" "default" {
         object_id_path = [123, 888]
       }
       policy_ids {
-        object_id_path = [456, 120]
+        object_id_path = [2,5,29,25]
       }
       ca_options {
         is_ca = true

google-beta/resource_privateca_certificate.go

@@ -86,7 +86,7 @@ running 'gcloud privateca locations list'.`,
 										Required:     true,
 										ForceNew:     true,
 										ValidateFunc: validation.StringInSlice([]string{"KEY_TYPE_UNSPECIFIED", "PEM"}, false),
-										Description:  `Types of public keys that are supported. At a minimum, we support RSA, for the key sizes or curves listed: https://cloud.google.com/kms/docs/algorithms#asymmetric_signing_algorithms Possible values: ["KEY_TYPE_UNSPECIFIED", "PEM"]`,
+										Description:  `The format of the public key. Currently, only PEM format is supported. Possible values: ["KEY_TYPE_UNSPECIFIED", "PEM"]`,
 									},
 									"key": {
 										Type:        schema.TypeString,
@@ -701,7 +701,7 @@ fractional digits, terminated by 's'. Example: "3.5s".`,
 									"format": {
 										Type:        schema.TypeString,
 										Computed:    true,
-										Description: `Types of public keys that are supported. At a minimum, we support RSA, for the key sizes or curves listed: https://cloud.google.com/kms/docs/algorithms#asymmetric_signing_algorithms`,
+										Description: `The format of the public key. Currently, only PEM format is supported.`,
 									},
 									"key": {
 										Type:        schema.TypeString,

google-beta/resource_privateca_certificate_authority.go

@@ -482,7 +482,7 @@ such as the CA certificate and CRLs. This must be a bucket name, without any pre
 my-bucket, you would simply specify 'my-bucket'. If not specified, a managed bucket will be
 created.`,
 			},
-			"ignore_active_certificates": {
+			"ignore_active_certificates_on_deletion": {
 				Type:     schema.TypeBool,
 				Optional: true,
 				ForceNew: true,
@@ -509,16 +509,6 @@ An object containing a list of "key": value pairs. Example: { "name": "wrench",
 fractional digits, terminated by 's'. Example: "3.5s".`,
 				Default: "315360000s",
 			},
-			"tier": {
-				Type:         schema.TypeString,
-				Optional:     true,
-				ForceNew:     true,
-				ValidateFunc: validation.StringInSlice([]string{"ENTERPRISE", "DEVOPS", ""}, false),
-				Description: `The Tier of this CertificateAuthority. 'ENTERPRISE' Certificate Authorities track
-server side certificates issued, and support certificate revocation. For more details,
-please check the [associated documentation](https://cloud.google.com/certificate-authority-service/docs/tiers). Default value: "ENTERPRISE" Possible values: ["ENTERPRISE", "DEVOPS"]`,
-				Default: "ENTERPRISE",
-			},
 			"type": {
 				Type:         schema.TypeString,
 				Optional:     true,
@@ -615,12 +605,6 @@ func resourcePrivatecaCertificateAuthorityCreate(d *schema.ResourceData, meta in
 	} else if v, ok := d.GetOkExists("type"); !isEmptyValue(reflect.ValueOf(typeProp)) && (ok || !reflect.DeepEqual(v, typeProp)) {
 		obj["type"] = typeProp
 	}
-	tierProp, err := expandPrivatecaCertificateAuthorityTier(d.Get("tier"), d, config)
-	if err != nil {
-		return err
-	} else if v, ok := d.GetOkExists("tier"); !isEmptyValue(reflect.ValueOf(tierProp)) && (ok || !reflect.DeepEqual(v, tierProp)) {
-		obj["tier"] = tierProp
-	}
 	configProp, err := expandPrivatecaCertificateAuthorityConfig(d.Get("config"), d, config)
 	if err != nil {
 		return err
@@ -726,6 +710,13 @@ func resourcePrivatecaCertificateAuthorityCreate(d *schema.ResourceData, meta in
 		return fmt.Errorf("Error enabling CertificateAuthority: %s", err)
 	}
 
+	err = privatecaOperationWaitTimeWithResponse(
+		config, res, &opRes, project, "Enabling CertificateAuthority", userAgent,
+		d.Timeout(schema.TimeoutCreate))
+	if err != nil {
+		return fmt.Errorf("Error waiting to enable CertificateAuthority: %s", err)
+	}
+
 	log.Printf("[DEBUG] Finished creating CertificateAuthority %q: %#v", d.Id(), res)
 
 	return resourcePrivatecaCertificateAuthorityRead(d, meta)
@@ -783,9 +774,6 @@ func resourcePrivatecaCertificateAuthorityRead(d *schema.ResourceData, meta inte
 	if err := d.Set("type", flattenPrivatecaCertificateAuthorityType(res["type"], d, config)); err != nil {
 		return fmt.Errorf("Error reading CertificateAuthority: %s", err)
 	}
-	if err := d.Set("tier", flattenPrivatecaCertificateAuthorityTier(res["tier"], d, config)); err != nil {
-		return fmt.Errorf("Error reading CertificateAuthority: %s", err)
-	}
 	if err := d.Set("config", flattenPrivatecaCertificateAuthorityConfig(res["config"], d, config)); err != nil {
 		return fmt.Errorf("Error reading CertificateAuthority: %s", err)
 	}
@@ -835,7 +823,7 @@ func resourcePrivatecaCertificateAuthorityDelete(d *schema.ResourceData, meta in
 	}
 	billingProject = project
 
-	url, err := replaceVars(d, config, "{{PrivatecaBasePath}}projects/{{project}}/locations/{{location}}/caPools/{{pool}}/certificateAuthorities/{{certificate_authority_id}}?ignoreActiveCertificates={{ignore_active_certificates}}")
+	url, err := replaceVars(d, config, "{{PrivatecaBasePath}}projects/{{project}}/locations/{{location}}/caPools/{{pool}}/certificateAuthorities/{{certificate_authority_id}}?ignoreActiveCertificates={{ignore_active_certificates_on_deletion}}")
 	if err != nil {
 		return err
 	}
@@ -846,11 +834,18 @@ func resourcePrivatecaCertificateAuthorityDelete(d *schema.ResourceData, meta in
 		return err
 	}
 
-	log.Printf("[DEBUG] Enabling CertificateAuthority: %#v", obj)
+	log.Printf("[DEBUG] Disabling CertificateAuthority: %#v", obj)
 
-	_, err = sendRequest(config, "POST", billingProject, disableUrl, userAgent, nil)
+	res, err = sendRequest(config, "POST", billingProject, disableUrl, userAgent, nil)
 	if err != nil {
-		return fmt.Errorf("Error enabling CertificateAuthority: %s", err)
+		return fmt.Errorf("Error disabling CertificateAuthority: %s", err)
+	}
+
+	err = privatecaOperationWaitTimeWithResponse(
+		config, res, &opRes, project, "Disabling CertificateAuthority", userAgent,
+		d.Timeout(schema.TimeoutDelete))
+	if err != nil {
+		return fmt.Errorf("Error waiting to disable CertificateAuthority: %s", err)
 	}
 	log.Printf("[DEBUG] Deleting CertificateAuthority %q", d.Id())
 
@@ -904,10 +899,6 @@ func flattenPrivatecaCertificateAuthorityType(v interface{}, d *schema.ResourceD
 	return v
 }
 
-func flattenPrivatecaCertificateAuthorityTier(v interface{}, d *schema.ResourceData, config *Config) interface{} {
-	return v
-}
-
 func flattenPrivatecaCertificateAuthorityConfig(v interface{}, d *schema.ResourceData, config *Config) interface{} {
 	if v == nil {
 		return nil
@@ -1129,10 +1120,6 @@ func expandPrivatecaCertificateAuthorityType(v interface{}, d TerraformResourceD
 	return v, nil
 }
 
-func expandPrivatecaCertificateAuthorityTier(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) {
-	return v, nil
-}
-
 func expandPrivatecaCertificateAuthorityConfig(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) {
 	l := v.([]interface{})
 	if len(l) == 0 || l[0] == nil {

google-beta/resource_privateca_certificate_authority_generated_test.go

@@ -43,7 +43,7 @@ func TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityBasicExam
 				ResourceName:            "google_privateca_certificate_authority.default",
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"ignore_active_certificates", "location", "certificate_authority_id", "pool"},
+				ImportStateVerifyIgnore: []string{"ignore_active_certificates_on_deletion", "location", "certificate_authority_id", "pool"},
 			},
 		},
 	})
@@ -101,7 +101,7 @@ resource "google_privateca_certificate_authority" "default" {
 `, context)
 }
 
-func TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityCmekExample(t *testing.T) {
+func TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityByoKeyExample(t *testing.T) {
 	skipIfVcr(t)
 	t.Parallel()
 
@@ -117,19 +117,19 @@ func TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityCmekExamp
 		CheckDestroy: testAccCheckPrivatecaCertificateAuthorityDestroyProducer(t),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityCmekExample(context),
+				Config: testAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityByoKeyExample(context),
 			},
 			{
 				ResourceName:            "google_privateca_certificate_authority.default",
 				ImportState:             true,
 				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"ignore_active_certificates", "location", "certificate_authority_id", "pool"},
+				ImportStateVerifyIgnore: []string{"ignore_active_certificates_on_deletion", "location", "certificate_authority_id", "pool"},
 			},
 		},
 	})
 }
 
-func testAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityCmekExample(context map[string]interface{}) string {
+func testAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityByoKeyExample(context map[string]interface{}) string {
 	return Nprintf(`
 resource "google_project_service_identity" "privateca_sa" {
   service = "privateca.googleapis.com"

google-beta/resource_privateca_certificate_generated_test.go

@@ -56,8 +56,7 @@ resource "google_privateca_certificate_authority" "test-ca" {
   certificate_authority_id = "tf-test-my-certificate-authority%{random_suffix}"
   location = "us-central1"
   pool = "%{pool}"
-  tier = "ENTERPRISE"
-  ignore_active_certificates = true
+  ignore_active_certificates_on_deletion = true
   config {
     subject_config {
       subject {
@@ -167,7 +166,6 @@ resource "google_privateca_certificate_authority" "test-ca" {
   pool = "%{pool}"
   certificate_authority_id = "tf-test-my-certificate-authority%{random_suffix}"
   location = "us-central1"
-  tier = "ENTERPRISE"
   config {
     subject_config {
       subject {

website/docs/r/privateca_ca_pool.html.markdown

@@ -259,12 +259,12 @@ The `allowed_issuance_modes` block supports:
 The `identity_constraints` block supports:
 
 * `allow_subject_passthrough` -
-  (Optional)
+  (Required)
   If this is set, the Subject field may be copied from a certificate request into the signed certificate.
   Otherwise, the requested Subject will be discarded.
 
 * `allow_subject_alt_names_passthrough` -
-  (Optional)
+  (Required)
   If this is set, the SubjectAltNames extension may be copied from a certificate request into the signed certificate.
   Otherwise, the requested SubjectAltNames will be discarded.
 

website/docs/r/privateca_certificate.html.markdown

@@ -43,8 +43,7 @@ resource "google_privateca_certificate_authority" "test-ca" {
   certificate_authority_id = "my-certificate-authority"
   location = "us-central1"
   pool = ""
-  tier = "ENTERPRISE"
-  ignore_active_certificates = true
+  ignore_active_certificates_on_deletion = true
   config {
     subject_config {
       subject {
@@ -132,7 +131,6 @@ resource "google_privateca_certificate_authority" "test-ca" {
   pool = ""
   certificate_authority_id = "my-certificate-authority"
   location = "us-central1"
-  tier = "ENTERPRISE"
   config {
     subject_config {
       subject {
@@ -563,7 +561,7 @@ The `public_key` block supports:
 
 * `format` -
   (Required)
-  Types of public keys that are supported. At a minimum, we support RSA, for the key sizes or curves listed: https://cloud.google.com/kms/docs/algorithms#asymmetric_signing_algorithms
+  The format of the public key. Currently, only PEM format is supported.
   Possible values are `KEY_TYPE_UNSPECIFIED` and `PEM`.
 
 ## Attributes Reference
@@ -819,7 +817,7 @@ The `public_key` block contains:
   Required. A public key. When this is specified in a request, the padding and encoding can be any of the options described by the respective 'KeyType' value. When this is generated by the service, it will always be an RFC 5280 SubjectPublicKeyInfo structure containing an algorithm identifier and a key. A base64-encoded string.
 
 * `format` -
-  Types of public keys that are supported. At a minimum, we support RSA, for the key sizes or curves listed: https://cloud.google.com/kms/docs/algorithms#asymmetric_signing_algorithms
+  The format of the public key. Currently, only PEM format is supported.
 
 The `subject_key_id` block contains:
 

website/docs/r/privateca_certificate_authority.html.markdown

@@ -90,11 +90,11 @@ resource "google_privateca_certificate_authority" "default" {
 }
 ```
 <div class = "oics-button" style="float: right; margin: 0 0 -15px">
-  <a href="https://console.cloud.google.com/cloudshell/open?cloudshell_git_repo=https%3A%2F%2Fgithub.com%2Fterraform-google-modules%2Fdocs-examples.git&cloudshell_working_dir=privateca_certificate_authority_cmek&cloudshell_image=gcr.io%2Fgraphite-cloud-shell-images%2Fterraform%3Alatest&open_in_editor=main.tf&cloudshell_print=.%2Fmotd&cloudshell_tutorial=.%2Ftutorial.md" target="_blank">
+  <a href="https://console.cloud.google.com/cloudshell/open?cloudshell_git_repo=https%3A%2F%2Fgithub.com%2Fterraform-google-modules%2Fdocs-examples.git&cloudshell_working_dir=privateca_certificate_authority_byo_key&cloudshell_image=gcr.io%2Fgraphite-cloud-shell-images%2Fterraform%3Alatest&open_in_editor=main.tf&cloudshell_print=.%2Fmotd&cloudshell_tutorial=.%2Ftutorial.md" target="_blank">
     <img alt="Open in Cloud Shell" src="//gstatic.com/cloudssh/images/open-btn.svg" style="max-height: 44px; margin: 32px auto; max-width: 100%;">
   </a>
 </div>
-## Example Usage - Privateca Certificate Authority Cmek
+## Example Usage - Privateca Certificate Authority Byo Key
 
 
 ```hcl
@@ -445,7 +445,7 @@ The `key_spec` block supports:
 - - -
 
 
-* `ignore_active_certificates` -
+* `ignore_active_certificates_on_deletion` -
   (Optional)
   This field allows the CA to be deleted even if the CA has active certs. Active certs include both unrevoked and unexpired certs.
   Use with care. Defaults to `false`.
@@ -459,14 +459,6 @@ The `key_spec` block supports:
   Default value is `SELF_SIGNED`.
   Possible values are `SELF_SIGNED` and `SUBORDINATE`.
 
-* `tier` -
-  (Optional)
-  The Tier of this CertificateAuthority. `ENTERPRISE` Certificate Authorities track
-  server side certificates issued, and support certificate revocation. For more details,
-  please check the [associated documentation](https://cloud.google.com/certificate-authority-service/docs/tiers).
-  Default value is `ENTERPRISE`.
-  Possible values are `ENTERPRISE` and `DEVOPS`.
-
 * `lifetime` -
   (Optional)
   The desired lifetime of the CA certificate. Used to create the "notBeforeTime" and