Merge pull request #341 from ns1-terraform/PENG-6067/fix-apikey-state

Fix to prevent api key from being removed from the state file when the key is updated
ns1-terraform · Feb 19, 2025 · 8cc2b0a · 8cc2b0a
2 parents b7986ca + 8488269
commit 8cc2b0a
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 2.5.2 (February 19, 2025)
+BUGFIX
+* Keep API key in state file
+
 ## 2.5.1 (February 5, 2025)
 ENHANCEMENTS
 * Allow underscore for non-HTTPS redirects

diff --git a/ns1/config.go b/ns1/config.go
@@ -19,7 +19,7 @@ import (
 )
 
 var (
-	clientVersion     = "2.5.1"
+	clientVersion     = "2.5.2"
 	providerUserAgent = "tf-ns1" + "/" + clientVersion
 	defaultRetryMax   = 3
 )

diff --git a/ns1/resource_apikey.go b/ns1/resource_apikey.go
@@ -59,11 +59,16 @@ func apikeyResource() *schema.Resource {
 func apikeyToResourceData(d *schema.ResourceData, k *account.APIKey) error {
 	d.SetId(k.ID)
 	d.Set("name", k.Name)
-	d.Set("key", k.Key)
 	d.Set("teams", k.TeamIDs)
 	d.Set("ip_whitelist", k.IPWhitelist)
 	d.Set("ip_whitelist_strict", k.IPWhitelistStrict)
 	permissionsToResourceData(d, k.Permissions)
+
+	// keep the existing key in the state file when there's no key in the response
+	if k.Key != "" {
+		d.Set("key", k.Key)
+	}
+
 	return nil
 }
 

diff --git a/ns1/resource_apikey_test.go b/ns1/resource_apikey_test.go
@@ -29,13 +29,15 @@ func TestAccAPIKey_basic(t *testing.T) {
 					testAccCheckAPIKeyExists("ns1_apikey.it", &apiKey),
 					testAccCheckAPIKeyName(&apiKey, name),
 					testAccCheckAPIKeyIPWhitelists(&apiKey, []string{"1.1.1.1", "2.2.2.2"}),
+					testAccCheckAPIKeyNotEmpty(&apiKey),
 					resource.TestCheckResourceAttr("ns1_apikey.it", "ip_whitelist_strict", "true"),
 				),
 			},
 			{
-				ResourceName:      "ns1_apikey.it",
-				ImportState:       true,
-				ImportStateVerify: true,
+				ResourceName:            "ns1_apikey.it",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"key"}, // importing existing key won't have the key
 			},
 		},
 	})
@@ -57,6 +59,7 @@ func TestAccAPIKey_updated(t *testing.T) {
 					testAccCheckAPIKeyExists("ns1_apikey.it", &apiKey),
 					testAccCheckAPIKeyName(&apiKey, name),
 					testAccCheckAPIKeyIPWhitelists(&apiKey, []string{"1.1.1.1", "2.2.2.2"}),
+					testAccCheckAPIKeyNotEmpty(&apiKey),
 					resource.TestCheckResourceAttr("ns1_apikey.it", "ip_whitelist_strict", "true"),
 				),
 			},
@@ -66,13 +69,15 @@ func TestAccAPIKey_updated(t *testing.T) {
 					testAccCheckAPIKeyExists("ns1_apikey.it", &apiKey),
 					testAccCheckAPIKeyName(&apiKey, updatedName),
 					testAccCheckAPIKeyIPWhitelists(&apiKey, []string{}),
+					testAccCheckAPIKeyNotEmpty(&apiKey),
 					resource.TestCheckResourceAttr("ns1_apikey.it", "ip_whitelist_strict", "false"),
 				),
 			},
 			{
-				ResourceName:      "ns1_apikey.it",
-				ImportState:       true,
-				ImportStateVerify: true,
+				ResourceName:            "ns1_apikey.it",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"key"}, // importing existing key won't have the key
 			},
 		},
 	})
@@ -122,14 +127,14 @@ func TestAccAPIKey_teamKey(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAPIKeyExists("ns1_apikey.it", &apiKey),
 					testAccCheckAPIKeyName(&apiKey, name),
-					// ensure Key attribute is populated on create of apikey joined to team
-					resource.TestCheckResourceAttrSet("ns1_apikey.it", "key"),
+					testAccCheckAPIKeyNotEmpty(&apiKey),
 				),
 			},
 			{
-				ResourceName:      "ns1_apikey.it",
-				ImportState:       true,
-				ImportStateVerify: true,
+				ResourceName:            "ns1_apikey.it",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"key"}, // importing existing key won't have the key
 			},
 		},
 	})
@@ -150,20 +155,23 @@ func TestAccAPIKey_permissions(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAPIKeyExists("ns1_apikey.it", &apiKey),
 					testAccCheckAPIKeyName(&apiKey, name),
+					testAccCheckAPIKeyNotEmpty(&apiKey),
 				),
 			},
 			{
 				Config: testAccAPIKeyPermissionsOnTeam(rString),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAPIKeyExists("ns1_apikey.it", &apiKey),
 					testAccCheckAPIKeyName(&apiKey, name),
+					testAccCheckAPIKeyNotEmpty(&apiKey),
 				),
 			},
 			{
 				Config: testAccAPIKeyPermissionsNoTeam(rString),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAPIKeyExists("ns1_apikey.it", &apiKey),
 					testAccCheckAPIKeyName(&apiKey, name),
+					testAccCheckAPIKeyNotEmpty(&apiKey),
 					// The key should still have this permission, it would have inherited it from the team.
 					resource.TestCheckResourceAttr("ns1_apikey.it", "account_manage_account_settings", "true"),
 					resource.TestCheckResourceAttr("ns1_apikey.it", "account_manage_ip_whitelist", "true"),
@@ -175,15 +183,17 @@ func TestAccAPIKey_permissions(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAPIKeyExists("ns1_apikey.it", &apiKey),
 					testAccCheckAPIKeyName(&apiKey, name),
+					testAccCheckAPIKeyNotEmpty(&apiKey),
 					// But if an apply is ran again, the permission will be removed.
 					resource.TestCheckResourceAttr("ns1_apikey.it", "account_manage_account_settings", "false"),
 					resource.TestCheckResourceAttr("ns1_apikey.it", "account_manage_ip_whitelist", "false"),
 				),
 			},
 			{
-				ResourceName:      "ns1_apikey.it",
-				ImportState:       true,
-				ImportStateVerify: true,
+				ResourceName:            "ns1_apikey.it",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"key"}, // importing existing key won't have the key
 			},
 		},
 	})
@@ -277,6 +287,24 @@ func testAccManualDeleteAPIKey(apiKey *account.APIKey) func() {
 	}
 }
 
+func testAccCheckAPIKeyNotEmpty(k *account.APIKey) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		var key string
+
+		for _, rs := range s.RootModule().Resources {
+			if rs.Primary.ID == k.ID {
+				key = rs.Primary.Attributes["key"]
+			}
+		}
+
+		if key == "" {
+			return fmt.Errorf("key should not be empty string")
+		}
+
+		return nil
+	}
+}
+
 func testAccAPIKeyBasic(apiKeyName string) string {
 	return fmt.Sprintf(`resource "ns1_apikey" "it" {
   name = "%s"

diff --git a/website/docs/r/apikey.html.markdown b/website/docs/r/apikey.html.markdown
@@ -87,6 +87,8 @@ In addition to all arguments above, the following attributes are exported:
 
 ## Import
 
+-> Imported keys will not have their key stored in the state file.
+
 `terraform import ns1_apikey`
 
 So for the example above: