From 15ff09705bd79fdabb08d2ffd1b8d48ef6e6922d Mon Sep 17 00:00:00 2001
From: Theron Voran <tvoran@users.noreply.github.com>
Date: Thu, 12 Aug 2021 14:17:30 -0700
Subject: [PATCH] Error handling for identity oidc vault calls (#1142)

Checks the err from `identityOidcKeyApiWrite()` and adds tests to
exercise the error handling. Updates allowed client id test to satisfy
rotation_period and verification_ttl restrictions that are now
enforced in Vault 1.8.1.
---
 vault/resource_identity_oidc_key.go           |  8 +++++--
 ...dentity_oidc_key_allowed_client_id_test.go | 13 ++++++++++--
 vault/resource_identity_oidc_key_test.go      | 21 +++++++++++++++++++
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/vault/resource_identity_oidc_key.go b/vault/resource_identity_oidc_key.go
index 0dd4be066..1f914fd2a 100644
--- a/vault/resource_identity_oidc_key.go
+++ b/vault/resource_identity_oidc_key.go
@@ -93,7 +93,9 @@ func identityOidcKeyCreate(d *schema.ResourceData, meta interface{}) error {
 	data := make(map[string]interface{})
 
 	identityOidcKeyUpdateFields(d, data)
-	identityOidcKeyApiWrite(name, data, client)
+	if err := identityOidcKeyApiWrite(name, data, client); err != nil {
+		return err
+	}
 
 	d.SetId(name)
 
@@ -113,7 +115,9 @@ func identityOidcKeyUpdate(d *schema.ResourceData, meta interface{}) error {
 	data := map[string]interface{}{}
 
 	identityOidcKeyUpdateFields(d, data)
-	identityOidcKeyApiWrite(name, data, client)
+	if err := identityOidcKeyApiWrite(name, data, client); err != nil {
+		return err
+	}
 
 	return identityOidcKeyRead(d, meta)
 }
diff --git a/vault/resource_identity_oidc_key_allowed_client_id_test.go b/vault/resource_identity_oidc_key_allowed_client_id_test.go
index 437dc646c..a6c297e51 100644
--- a/vault/resource_identity_oidc_key_allowed_client_id_test.go
+++ b/vault/resource_identity_oidc_key_allowed_client_id_test.go
@@ -22,6 +22,9 @@ func TestAccIdentityOidcKeyAllowedClientId(t *testing.T) {
 			{
 				Config: testAccIdentityOidcKeyAllowedClientIdConfig(name),
 				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "rotation_period", "86400"),
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "verification_ttl", "86400"),
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "algorithm", "RS256"),
 					testAccIdentityOidcKeyAllowedClientIdCheckAttrs("vault_identity_oidc_key_allowed_client_id.role_one", 3),
 					testAccIdentityOidcKeyAllowedClientIdCheckAttrs("vault_identity_oidc_key_allowed_client_id.role_two", 3),
 					testAccIdentityOidcKeyAllowedClientIdCheckAttrs("vault_identity_oidc_key_allowed_client_id.role_three", 3),
@@ -30,12 +33,18 @@ func TestAccIdentityOidcKeyAllowedClientId(t *testing.T) {
 			{
 				Config: testAccIdentityOidcKeyAllowedClientIdRemove(name),
 				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "rotation_period", "86401"),
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "verification_ttl", "86401"),
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "algorithm", "RS256"),
 					testAccIdentityOidcKeyAllowedClientIdCheckAttrs("vault_identity_oidc_key_allowed_client_id.role_one", 1),
 				),
 			},
 			{
 				Config: testAccIdentityOidcKeyAllowedClientIdRecreate(name),
 				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "rotation_period", "86400"),
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "verification_ttl", "86400"),
+					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "algorithm", "RS256"),
 					testAccIdentityOidcKeyAllowedClientIdCheckAttrs("vault_identity_oidc_key_allowed_client_id.role", 1),
 				),
 			},
@@ -152,8 +161,8 @@ resource "vault_identity_oidc_key" "key" {
   name = "%s"
 	algorithm = "RS256"
 
-	rotation_period  = 3600
-	verification_ttl = 3600
+	rotation_period  = 86401
+	verification_ttl = 86401
 }
 
 resource "vault_identity_oidc_role" "role_one" {
diff --git a/vault/resource_identity_oidc_key_test.go b/vault/resource_identity_oidc_key_test.go
index 9b6d55d11..c0b97d670 100644
--- a/vault/resource_identity_oidc_key_test.go
+++ b/vault/resource_identity_oidc_key_test.go
@@ -3,6 +3,7 @@ package vault
 import (
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"strconv"
 	"strings"
 	"testing"
@@ -21,6 +22,11 @@ func TestAccIdentityOidcKey(t *testing.T) {
 		Providers:    testProviders,
 		CheckDestroy: testAccCheckIdentityOidcKeyDestroy,
 		Steps: []resource.TestStep{
+			{
+				// Test a create failure
+				Config:      testAccIdentityOidcKeyConfig_bad(key),
+				ExpectError: regexp.MustCompile(`unknown signing algorithm "RS123"`),
+			},
 			{
 				Config: testAccIdentityOidcKeyConfig(key),
 				Check: resource.ComposeTestCheckFunc(
@@ -75,6 +81,11 @@ func TestAccIdentityOidcKeyUpdate(t *testing.T) {
 					resource.TestCheckResourceAttr("vault_identity_oidc_key.key", "allowed_client_ids.#", "0"),
 				),
 			},
+			{
+				// Test an update failure
+				Config:      testAccIdentityOidcKeyConfig_bad(key),
+				ExpectError: regexp.MustCompile(`unknown signing algorithm "RS123"`),
+			},
 		},
 	})
 }
@@ -204,6 +215,16 @@ resource "vault_identity_oidc_key" "key" {
 }`, entityName)
 }
 
+func testAccIdentityOidcKeyConfig_bad(entityName string) string {
+	return fmt.Sprintf(`
+resource "vault_identity_oidc_key" "key" {
+  name = "%s"
+	algorithm = "RS123"
+
+	allowed_client_ids = []
+}`, entityName)
+}
+
 func testAccIdentityOidcKeyConfigUpdate(entityName string) string {
 	return fmt.Sprintf(`
 resource "vault_identity_oidc_key" "key" {