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.

Sign up for GitHub

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 path to invoke update without determining changed CRN data, add validator for name of secrets #4859

Merged
merged 12 commits into from
Oct 25, 2023
Merged

Add path to invoke update without determining changed CRN data, add validator for name of secrets #4859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
59ca57e
update + validate
Oct 3, 2023
bd91c17
Merge branch 'IBM-Cloud:master' into terraform-ingress-update-secret-…
jared-hayes-dev Oct 11, 2023
13cba28
invalid name tests
Oct 16, 2023
49702b6
updated to add new tf only flag for update + tests
Oct 17, 2023
58180d6
update false positives
Oct 17, 2023
28ddaa2
update secret int
Oct 18, 2023
cdbd195
update docs
Oct 18, 2023
e24be7d
fetch upstream
Oct 18, 2023
3b0bce8
secrets audit
Oct 18, 2023
3a75909
pr comments
Oct 19, 2023
45555b7
updated tests
Oct 24, 2023
d815ea8
Merge branch 'master' into terraform-ingress-update-secret-validate-name
hkantare Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions .secrets.baseline
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"files": "go.mod|go.sum|.*.map|^.secrets.baseline$",
"lines": null
},
"generated_at": "2023-10-17T18:07:42Z",
"generated_at": "2023-10-18T18:46:12Z",
"plugins_used": [
{
"name": "AWSKeyDetector"
Expand Down Expand Up @@ -792,23 +792,23 @@
"hashed_secret": "1f614c2eb6b3da22d89bd1b9fd47d7cb7c8fc670",
"is_secret": false,
"is_verified": false,
"line_number": 3248,
"line_number": 3250,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "7abfce65b8504403afc25c9790f358d513dfbcc6",
"is_secret": false,
"is_verified": false,
"line_number": 3261,
"line_number": 3263,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "0c2d85bf9a9b1579b16f220a4ea8c3d62b2e24b1",
"is_secret": false,
"is_verified": false,
"line_number": 3302,
"line_number": 3304,
"type": "Secret Keyword",
"verified_result": null
}
Expand Down Expand Up @@ -2772,7 +2772,17 @@
"hashed_secret": "b732fb611fd46a38e8667f9972e0cde777fbe37f",
"is_secret": false,
"is_verified": false,
"line_number": 373,
"line_number": 403,
"type": "Secret Keyword",
"verified_result": null
}
],
"ibm/service/kubernetes/resource_ibm_container_ingress_secret_opaque_test.go": [
{
"hashed_secret": "5df29b80b97dab81130058ac9af4a9b9d1c091b4",
"is_secret": false,
"is_verified": false,
"line_number": 247,
"type": "Secret Keyword",
"verified_result": null
}
Expand All @@ -2782,7 +2792,17 @@
"hashed_secret": "b732fb611fd46a38e8667f9972e0cde777fbe37f",
"is_secret": false,
"is_verified": false,
"line_number": 267,
"line_number": 292,
"type": "Secret Keyword",
"verified_result": null
}
],
"ibm/service/kubernetes/resource_ibm_container_ingress_secret_tls_test.go": [
{
"hashed_secret": "5df29b80b97dab81130058ac9af4a9b9d1c091b4",
"is_secret": false,
"is_verified": false,
"line_number": 216,
"type": "Secret Keyword",
"verified_result": null
}
Expand Down Expand Up @@ -2920,7 +2940,7 @@
"hashed_secret": "83747cea2b26d7652ed39218ddcdb1461c570535",
"is_secret": false,
"is_verified": false,
"line_number": 95,
"line_number": 94,
"type": "Hex High Entropy String",
"verified_result": null
}
Expand Down Expand Up @@ -3088,15 +3108,15 @@
"hashed_secret": "3046d9f6cfaaeea6eed9bb7a4ab010fe49b0cfd4",
"is_secret": false,
"is_verified": false,
"line_number": 231,
"line_number": 235,
"type": "Secret Keyword",
"verified_result": null
},
{
"hashed_secret": "b732fb611fd46a38e8667f9972e0cde777fbe37f",
"is_secret": false,
"is_verified": false,
"line_number": 410,
"line_number": 414,
"type": "Secret Keyword",
"verified_result": null
}
Expand Down Expand Up @@ -3244,7 +3264,7 @@
"hashed_secret": "347cd9c53ff77d41a7b22aa56c7b4efaf54658e3",
"is_secret": false,
"is_verified": false,
"line_number": 46,
"line_number": 48,
"type": "Secret Keyword",
"verified_result": null
}
Expand Down Expand Up @@ -3616,7 +3636,7 @@
"hashed_secret": "4d55af37dbbb6a42088d917caa1ca25428ec42c9",
"is_secret": false,
"is_verified": false,
"line_number": 788,
"line_number": 849,
"type": "Secret Keyword",
"verified_result": null
}
Expand Down
33 changes: 31 additions & 2 deletions ibm/service/kubernetes/resource_ibm_container_ingress_secret_opaque.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ func ResourceIBMContainerIngressSecretOpaque() *schema.Resource {
Required: true,
Description: "Secret name",
ForceNew: true,
ValidateFunc: validate.InvokeValidator(
"ibm_container_ingress_secret_opaque",
"secret_name"),
},
"secret_namespace": {
Type: schema.TypeString,
Expand Down Expand Up @@ -73,6 +76,16 @@ func ResourceIBMContainerIngressSecretOpaque() *schema.Resource {
Computed: true,
Description: "Status of the secret",
},
"update_secret": {
Type: schema.TypeInt,
Optional: true,
Description: "Updates secret from secrets manager if value is changed (increment each usage)",
},
Comment on lines +79 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

update_secret is never set in the code with d.Set. if I get it right, terraform will detect this field as changed on every apply, if the field is not empty.
this can be verified with a test, where you apply the same testAccCheckIBMContainerIngressSecretOpaqueForceUpdate configuration twice.

you can to use d.Set and update the state after the ingressAPI.UpdateIngressSecret .

( or if this behaviour works for you, the field can be modified to a boolean. )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was unable to replicate the behavior you mentioned either by running the test in the code with the same config again or by manually using my own terraform plan/resources. I see the update_secret field reflected in the state file and terraform doesn't execute an update on a repeat apply of the same config should the variable remain unchanged. I also took a look at the above mentioned field retry_patch_version whose pattern update_secret was modeled after and I didn't see any instances of that implementation using the d.set either. Although even in the case it did update every time that would be acceptable behavior. Please let me know if there are any other concerns.

I also went ahead and updated the tests to validate this behavior:

make testacc TEST=./ibm/service/kubernetes TESTARGS='-run=TestAccIBMContainerIngressSecretOpaque_ForceUpdate'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./ibm/service/kubernetes -v -run=TestAccIBMContainerIngressSecretOpaque_ForceUpdate -timeout 700m
=== RUN   TestAccIBMContainerIngressSecretOpaque_ForceUpdate
--- PASS: TestAccIBMContainerIngressSecretOpaque_ForceUpdate (84.83s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes	86.414s

make testacc TEST=./ibm/service/kubernetes TESTARGS='-run=TestAccIBMContainerIngressSecretTLS_BasicForceUpdate'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./ibm/service/kubernetes -v -run=TestAccIBMContainerIngressSecretTLS_BasicForceUpdate -timeout 700m
=== RUN   TestAccIBMContainerIngressSecretTLS_BasicForceUpdate
--- PASS: TestAccIBMContainerIngressSecretTLS_BasicForceUpdate (63.88s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes	65.472s

Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't sure about this one.
thank you Jared for looking into it!

"last_updated_timestamp": {
Type: schema.TypeString,
Computed: true,
Description: "Timestamp secret was last updated",
},
"fields": {
Type: schema.TypeSet,
Required: true,
Expand Down Expand Up @@ -122,6 +135,16 @@ func ResourceIBMContainerIngressSecretOpaqueValidator() *validate.ResourceValida
CloudDataType: "cluster",
CloudDataRange: []string{"resolved_to:id"}})

validateSchema = append(validateSchema, validate.ValidateSchema{
Identifier: "secret_name",
ValidateFunctionIdentifier: validate.ValidateRegexpLen,
Type: validate.TypeString,
Required: true,
Regexp: `^([a-z0-9]([-a-z0-9]*[a-z0-9])?(.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)$`,
MinValueLength: 1,
MaxValueLength: 63,
})

iBMContainerIngressInstanceValidator := validate.ResourceValidator{ResourceName: "ibm_container_ingress_secret_opaque", Schema: validateSchema}
return &iBMContainerIngressInstanceValidator
}
Expand Down Expand Up @@ -205,6 +228,7 @@ func resourceIBMContainerIngressSecretOpaqueRead(d *schema.ResourceData, meta in
d.Set("persistence", ingressSecretConfig.Persistence)
d.Set("user_managed", ingressSecretConfig.UserManaged)
d.Set("status", ingressSecretConfig.Status)
d.Set("last_updated_timestamp", ingressSecretConfig.LastUpdatedTimestamp)

if len(ingressSecretConfig.Fields) > 0 {
d.Set("fields", flex.FlattenOpaqueSecret(ingressSecretConfig.Fields))
Expand Down Expand Up @@ -261,6 +285,7 @@ func resourceIBMContainerIngressSecretOpaqueUpdate(d *schema.ResourceData, meta
Namespace: secretNamespace,
}

ingressAPI := ingressClient.Ingresses()
if d.HasChange("fields") {
oldList, newList := d.GetChange("fields")

Expand All @@ -276,7 +301,6 @@ func resourceIBMContainerIngressSecretOpaqueUpdate(d *schema.ResourceData, meta
remove := os.Difference(ns).List()
add := ns.Difference(os).List()

ingressAPI := ingressClient.Ingresses()
if len(remove) > 0 {
actualSecret, err := ingressAPI.GetIngressSecret(cluster, secretName, secretNamespace)
if err != nil {
Expand Down Expand Up @@ -342,8 +366,13 @@ func resourceIBMContainerIngressSecretOpaqueUpdate(d *schema.ResourceData, meta
return err
}
}
} else if d.HasChange("update_secret") {
// user wants to force an upstream secret update from secrets manager onto kube cluster w/out changing crn
_, err = ingressAPI.UpdateIngressSecret(params)
if err != nil {
return err
}
}

return resourceIBMContainerIngressSecretOpaqueRead(d, meta)
}

Expand Down
147 changes: 146 additions & 1 deletion ibm/service/kubernetes/resource_ibm_container_ingress_secret_opaque_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package kubernetes_test

import (
"fmt"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -70,7 +71,124 @@ func TestAccIBMContainerIngressSecretOpaque_Basic(t *testing.T) {
ResourceName: "ibm_container_ingress_secret_opaque.secret",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"region", "issuer_name"},
ImportStateVerifyIgnore: []string{"region", "issuer_name", "update_secret"},
},
},
})
}

func TestAccIBMContainerIngressSecretOpaque_InvalidName(t *testing.T) {
secretName := fmt.Sprintf(")-tf-container-ingress-secret-name-opaque-%d", acctest.RandIntRange(10, 100))

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
Providers: acc.TestAccProviders,
CheckDestroy: testAccCheckIBMContainerIngressSecretDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckIBMContainerIngressSecretOpaqueBasic(secretName),
ExpectError: regexp.MustCompile(".*should match regexp"),
},
},
})
}

func TestAccIBMContainerIngressSecretOpaque_ForceUpdate(t *testing.T) {
secretName := fmt.Sprintf("tf-container-ingress-secret-name-opaque-update-%d", acctest.RandIntRange(10, 100))

var originalTS string

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
Providers: acc.TestAccProviders,
CheckDestroy: testAccCheckIBMContainerIngressSecretDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckIBMContainerIngressSecretOpaqueForceUpdateCreate(secretName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "cluster", acc.ClusterName),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "secret_name", secretName),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "secret_namespace", "ibm-cert-store"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "persistence", "true"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "type", "Opaque"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "fields.#", "1"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "user_managed", "true"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "status", "created"),
resource.TestCheckResourceAttrWith("ibm_container_ingress_secret_opaque.secret", "last_updated_timestamp", func(value string) error {
originalTS = value
return nil
}),
),
},
{
Config: testAccCheckIBMContainerIngressSecretOpaqueForceUpdate(secretName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "cluster", acc.ClusterName),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "secret_name", secretName),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "secret_namespace", "ibm-cert-store"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "persistence", "true"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "type", "Opaque"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "fields.#", "1"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "user_managed", "true"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "status", "created"),
resource.TestCheckResourceAttrWith("ibm_container_ingress_secret_opaque.secret", "last_updated_timestamp", func(value string) error {
if originalTS == value {
return fmt.Errorf("error timestamp not changed, indicates update didnt go through. original: %s, actual: %s", originalTS, value)
}
originalTS = value // check if another update will execute without a change to `update_secret`
return nil
}),
),
},
{
Config: testAccCheckIBMContainerIngressSecretOpaqueForceUpdate(secretName),
ExpectNonEmptyPlan: false,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "cluster", acc.ClusterName),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "secret_name", secretName),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "secret_namespace", "ibm-cert-store"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "persistence", "true"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "type", "Opaque"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "fields.#", "1"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "user_managed", "true"),
resource.TestCheckResourceAttr(
"ibm_container_ingress_secret_opaque.secret", "status", "created"),
resource.TestCheckResourceAttrWith("ibm_container_ingress_secret_opaque.secret", "last_updated_timestamp", func(value string) error {
if originalTS != value {
return fmt.Errorf("error timestamp has changed, indicates update was called again even though no modification of fields. exptected: %s, actual: %s", originalTS, value)
}
return nil
}),
),
},
{
ResourceName: "ibm_container_ingress_secret_opaque.secret",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"region", "issuer_name", "update_secret"},
},
},
})
Expand Down Expand Up @@ -134,3 +252,30 @@ resource "ibm_container_ingress_secret_opaque" "secret" {
}
}`, secretName, "ibm-cert-store", acc.ClusterName, true, acc.SecretCRN)
}

func testAccCheckIBMContainerIngressSecretOpaqueForceUpdateCreate(secretName string) string {
return fmt.Sprintf(`
resource "ibm_container_ingress_secret_opaque" "secret" {
secret_name = "%s"
secret_namespace = "%s"
cluster = "%s"
persistence = "%t"
fields {
crn = "%s"
}
}`, secretName, "ibm-cert-store", acc.ClusterName, true, acc.SecretCRN)
}

func testAccCheckIBMContainerIngressSecretOpaqueForceUpdate(secretName string) string {
return fmt.Sprintf(`
resource "ibm_container_ingress_secret_opaque" "secret" {
secret_name = "%s"
secret_namespace = "%s"
cluster = "%s"
persistence = "%t"
update_secret = "%d"
fields {
crn = "%s"
}
}`, secretName, "ibm-cert-store", acc.ClusterName, true, 1, acc.SecretCRN)
}
Loading
Loading