From 68bead4c2717a77f4da31a5f2e737f066f384fb9 Mon Sep 17 00:00:00 2001 From: Andrejs Mivreniks Date: Tue, 24 Dec 2024 20:29:57 +0200 Subject: [PATCH] Resolve failing tests --- example/main.tf | 18 ++---- keycloak/version.go | 16 +++++ ...ource_keycloak_authentication_execution.go | 4 ++ ..._keycloak_authentication_execution_test.go | 50 +++++++++++++--- ...ak_authentication_execution_config_test.go | 15 ++--- ..._keycloak_authentication_execution_test.go | 57 +++++++++++++++++- ...ce_keycloak_authentication_subflow_test.go | 59 +++++++++++++++++++ provider/test_utils.go | 11 ++++ 8 files changed, 201 insertions(+), 29 deletions(-) diff --git a/example/main.tf b/example/main.tf index 3aa2ae8a3..43890dff3 100644 --- a/example/main.tf +++ b/example/main.tf @@ -1020,9 +1020,7 @@ resource "keycloak_authentication_execution" "browser-copy-cookie" { parent_flow_alias = keycloak_authentication_flow.browser-copy-flow.alias authenticator = "auth-cookie" requirement = "ALTERNATIVE" - depends_on = [ - keycloak_authentication_execution.browser-copy-kerberos - ] + priority = 20 } resource "keycloak_authentication_execution" "browser-copy-kerberos" { @@ -1030,6 +1028,7 @@ resource "keycloak_authentication_execution" "browser-copy-kerberos" { parent_flow_alias = keycloak_authentication_flow.browser-copy-flow.alias authenticator = "auth-spnego" requirement = "DISABLED" + priority = 10 } resource "keycloak_authentication_execution" "browser-copy-idp-redirect" { @@ -1037,9 +1036,7 @@ resource "keycloak_authentication_execution" "browser-copy-idp-redirect" { parent_flow_alias = keycloak_authentication_flow.browser-copy-flow.alias authenticator = "identity-provider-redirector" requirement = "ALTERNATIVE" - depends_on = [ - keycloak_authentication_execution.browser-copy-cookie - ] + priority = 30 } resource "keycloak_authentication_subflow" "browser-copy-flow-forms" { @@ -1047,9 +1044,7 @@ resource "keycloak_authentication_subflow" "browser-copy-flow-forms" { parent_flow_alias = keycloak_authentication_flow.browser-copy-flow.alias alias = "browser-copy-flow-forms" requirement = "ALTERNATIVE" - depends_on = [ - keycloak_authentication_execution.browser-copy-idp-redirect - ] + priority = 40 } resource "keycloak_authentication_execution" "browser-copy-auth-username-password-form" { @@ -1057,6 +1052,7 @@ resource "keycloak_authentication_execution" "browser-copy-auth-username-passwor parent_flow_alias = keycloak_authentication_subflow.browser-copy-flow-forms.alias authenticator = "auth-username-password-form" requirement = "REQUIRED" + priority = 10 } resource "keycloak_authentication_execution" "browser-copy-otp" { @@ -1064,9 +1060,7 @@ resource "keycloak_authentication_execution" "browser-copy-otp" { parent_flow_alias = keycloak_authentication_subflow.browser-copy-flow-forms.alias authenticator = "auth-otp-form" requirement = "REQUIRED" - depends_on = [ - keycloak_authentication_execution.browser-copy-auth-username-password-form - ] + priority = 20 } resource "keycloak_authentication_execution_config" "config" { diff --git a/keycloak/version.go b/keycloak/version.go index b59d0b2ac..8bb14b0d7 100644 --- a/keycloak/version.go +++ b/keycloak/version.go @@ -74,3 +74,19 @@ func (keycloakClient *KeycloakClient) VersionIsLessThanOrEqualTo(ctx context.Con return keycloakClient.version.LessThanOrEqual(v), nil } + +func (keycloakClient *KeycloakClient) VersionIsLessThan(ctx context.Context, versionString Version) (bool, error) { + if keycloakClient.version == nil { + err := keycloakClient.login(ctx) + if err != nil { + return false, err + } + } + + v, err := version.NewVersion(string(versionString)) + if err != nil { + return false, nil + } + + return keycloakClient.version.LessThan(v), nil +} diff --git a/provider/data_source_keycloak_authentication_execution.go b/provider/data_source_keycloak_authentication_execution.go index 83dea70b6..adca81144 100644 --- a/provider/data_source_keycloak_authentication_execution.go +++ b/provider/data_source_keycloak_authentication_execution.go @@ -23,6 +23,10 @@ func dataSourceKeycloakAuthenticationExecution() *schema.Resource { Type: schema.TypeString, Required: true, }, + "priority": { + Type: schema.TypeInt, + Optional: true, + }, }, } } diff --git a/provider/data_source_keycloak_authentication_execution_test.go b/provider/data_source_keycloak_authentication_execution_test.go index ad788631d..034b3d1b3 100644 --- a/provider/data_source_keycloak_authentication_execution_test.go +++ b/provider/data_source_keycloak_authentication_execution_test.go @@ -8,9 +8,11 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/keycloak/terraform-provider-keycloak/keycloak" ) func TestAccKeycloakDataSourceAuthenticationExecution_basic(t *testing.T) { + skipIfVersionIsGreaterThanOrEqualTo(testCtx, t, keycloakClient, keycloak.Version_25) t.Parallel() parentFlowAlias := acctest.RandomWithPrefix("tf-acc") @@ -21,7 +23,7 @@ func TestAccKeycloakDataSourceAuthenticationExecution_basic(t *testing.T) { CheckDestroy: testAccCheckKeycloakAuthenticationExecutionConfigDestroy, Steps: []resource.TestStep{ { - Config: testDataSourceKeycloakAuthenticationExecution_basic(parentFlowAlias), + Config: testDataSourceKeycloakAuthenticationExecution_basic(parentFlowAlias, 10), Check: resource.ComposeTestCheckFunc( testAccCheckKeycloakAuthenticationExecutionExists("keycloak_authentication_execution.execution"), resource.TestCheckResourceAttrPair("keycloak_authentication_execution.execution", "id", "data.keycloak_authentication_execution.execution", "id"), @@ -35,6 +37,33 @@ func TestAccKeycloakDataSourceAuthenticationExecution_basic(t *testing.T) { }) } +func TestAccKeycloakDataSourceAuthenticationExecutionWithPriority_basic(t *testing.T) { + skipIfVersionIsLessThan(testCtx, t, keycloakClient, keycloak.Version_25) + t.Parallel() + + parentFlowAlias := acctest.RandomWithPrefix("tf-acc") + + resource.Test(t, resource.TestCase{ + ProviderFactories: testAccProviderFactories, + PreCheck: func() { testAccPreCheck(t) }, + CheckDestroy: testAccCheckKeycloakAuthenticationExecutionConfigDestroy, + Steps: []resource.TestStep{ + { + Config: testDataSourceKeycloakAuthenticationExecution_basic(parentFlowAlias, 10), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakAuthenticationExecutionExists("keycloak_authentication_execution.execution"), + resource.TestCheckResourceAttrPair("keycloak_authentication_execution.execution", "id", "data.keycloak_authentication_execution.execution", "id"), + resource.TestCheckResourceAttrPair("keycloak_authentication_execution.execution", "realm_id", "data.keycloak_authentication_execution.execution", "realm_id"), + resource.TestCheckResourceAttrPair("keycloak_authentication_execution.execution", "priority", "data.keycloak_authentication_execution.execution", "priority"), + resource.TestCheckResourceAttrPair("keycloak_authentication_execution.execution", "parent_flow_alias", "data.keycloak_authentication_execution.execution", "parent_flow_alias"), + resource.TestCheckResourceAttrPair("keycloak_authentication_execution.execution", "authenticator", "data.keycloak_authentication_execution.execution", "provider_id"), + testAccCheckDataKeycloakAuthenticationExecution("data.keycloak_authentication_execution.execution"), + ), + }, + }, + }) +} + func TestAccKeycloakDataSourceAuthenticationExecution_errorNoExecutions(t *testing.T) { t.Parallel() parentFlowAlias := acctest.RandomWithPrefix("tf-acc") @@ -45,7 +74,7 @@ func TestAccKeycloakDataSourceAuthenticationExecution_errorNoExecutions(t *testi CheckDestroy: testAccCheckKeycloakAuthenticationExecutionConfigDestroy, Steps: []resource.TestStep{ { - Config: testDataSourceKeycloakAuthenticationExecution_errorNoExecutions(parentFlowAlias), + Config: testDataSourceKeycloakAuthenticationExecution_errorNoExecutions(parentFlowAlias, 10), ExpectError: regexp.MustCompile("no authentication executions found for parent flow alias .*"), }, }, @@ -62,7 +91,7 @@ func TestAccKeycloakDataSourceAuthenticationExecution_errorWrongProviderId(t *te CheckDestroy: testAccCheckKeycloakAuthenticationExecutionConfigDestroy, Steps: []resource.TestStep{ { - Config: testDataSourceKeycloakAuthenticationExecution_errorWrongProviderId(parentFlowAlias, acctest.RandString(10)), + Config: testDataSourceKeycloakAuthenticationExecution_errorWrongProviderId(parentFlowAlias, acctest.RandString(10), 10), ExpectError: regexp.MustCompile("no authentication execution under parent flow alias .* with provider id .* found"), }, }, @@ -94,7 +123,7 @@ func testAccCheckDataKeycloakAuthenticationExecution(resourceName string) resour } } -func testDataSourceKeycloakAuthenticationExecution_basic(parentFlowAlias string) string { +func testDataSourceKeycloakAuthenticationExecution_basic(parentFlowAlias string, priority int) string { return fmt.Sprintf(` data "keycloak_realm" "realm" { realm = "%s" @@ -110,6 +139,7 @@ resource "keycloak_authentication_execution" "execution" { parent_flow_alias = keycloak_authentication_flow.flow.alias authenticator = "identity-provider-redirector" requirement = "REQUIRED" + priority = %d } data "keycloak_authentication_execution" "execution" { @@ -121,10 +151,10 @@ data "keycloak_authentication_execution" "execution" { keycloak_authentication_execution.execution, ] } - `, testAccRealm.Realm, parentFlowAlias) + `, testAccRealm.Realm, parentFlowAlias, priority) } -func testDataSourceKeycloakAuthenticationExecution_errorNoExecutions(parentFlowAlias string) string { +func testDataSourceKeycloakAuthenticationExecution_errorNoExecutions(parentFlowAlias string, priority int) string { return fmt.Sprintf(` data "keycloak_realm" "realm" { realm = "%s" @@ -139,15 +169,16 @@ data "keycloak_authentication_execution" "execution" { realm_id = data.keycloak_realm.realm.id parent_flow_alias = keycloak_authentication_flow.flow.alias provider_id = "foo" + priority = %d depends_on = [ keycloak_authentication_flow.flow, ] } - `, testAccRealm.Realm, parentFlowAlias) + `, testAccRealm.Realm, parentFlowAlias, priority) } -func testDataSourceKeycloakAuthenticationExecution_errorWrongProviderId(parentFlowAlias, providerId string) string { +func testDataSourceKeycloakAuthenticationExecution_errorWrongProviderId(parentFlowAlias, providerId string, priority int) string { return fmt.Sprintf(` data "keycloak_realm" "realm" { realm = "%s" @@ -163,6 +194,7 @@ resource "keycloak_authentication_execution" "execution" { parent_flow_alias = keycloak_authentication_flow.flow.alias authenticator = "identity-provider-redirector" requirement = "REQUIRED" + priority = %d } data "keycloak_authentication_execution" "execution" { @@ -174,5 +206,5 @@ data "keycloak_authentication_execution" "execution" { keycloak_authentication_execution.execution, ] } - `, testAccRealm.Id, parentFlowAlias, providerId) + `, testAccRealm.Id, parentFlowAlias, priority, providerId) } diff --git a/provider/resource_keycloak_authentication_execution_config_test.go b/provider/resource_keycloak_authentication_execution_config_test.go index e8a323b63..d985a5f98 100644 --- a/provider/resource_keycloak_authentication_execution_config_test.go +++ b/provider/resource_keycloak_authentication_execution_config_test.go @@ -26,7 +26,7 @@ func TestAccKeycloakAuthenticationExecutionConfig_basic(t *testing.T) { CheckDestroy: testAccCheckKeycloakAuthenticationExecutionConfigDestroy, Steps: []resource.TestStep{ { - Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAlias, configProviderOne), + Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAlias, configProviderOne, 10), Check: resource.ComposeTestCheckFunc( testAccCheckKeycloakAuthenticationExecutionConfigExists("keycloak_authentication_execution_config.config", &config1), resource.TestCheckResourceAttr("keycloak_authentication_execution_config.config", "realm_id", testAccRealm.Realm), @@ -36,7 +36,7 @@ func TestAccKeycloakAuthenticationExecutionConfig_basic(t *testing.T) { ), }, { - Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAlias, configProviderTwo), + Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAlias, configProviderTwo, 10), Check: resource.ComposeTestCheckFunc( testAccCheckKeycloakAuthenticationExecutionConfigExists("keycloak_authentication_execution_config.config", &config2), resource.TestCheckResourceAttr("keycloak_authentication_execution_config.config", "realm_id", testAccRealm.Realm), @@ -66,7 +66,7 @@ func TestAccKeycloakAuthenticationExecutionConfig_updateForcesNew(t *testing.T) CheckDestroy: testAccCheckKeycloakAuthenticationExecutionConfigDestroy, Steps: []resource.TestStep{ { - Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAliasOne, configProvider), + Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAliasOne, configProvider, 10), Check: resource.ComposeTestCheckFunc( testAccCheckKeycloakAuthenticationExecutionConfigExists("keycloak_authentication_execution_config.config", &config1), resource.TestCheckResourceAttr("keycloak_authentication_execution_config.config", "realm_id", testAccRealm.Realm), @@ -76,7 +76,7 @@ func TestAccKeycloakAuthenticationExecutionConfig_updateForcesNew(t *testing.T) ), }, { - Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAliasTwo, configProvider), + Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAliasTwo, configProvider, 10), Check: resource.ComposeTestCheckFunc( testAccCheckKeycloakAuthenticationExecutionConfigExists("keycloak_authentication_execution_config.config", &config2), resource.TestCheckResourceAttr("keycloak_authentication_execution_config.config", "realm_id", testAccRealm.Realm), @@ -103,7 +103,7 @@ func TestAccKeycloakAuthenticationExecutionConfig_import(t *testing.T) { CheckDestroy: testAccCheckKeycloakAuthenticationExecutionConfigDestroy, Steps: []resource.TestStep{ { - Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAlias, configProvider), + Config: testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAlias, configProvider, 10), }, { ResourceName: "keycloak_authentication_execution_config.config", @@ -184,7 +184,7 @@ func testAccCheckKeycloakAuthenticationExecutionConfigForceNew(old, new *keycloa } } -func testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAlias, configProvider string) string { +func testAccKeycloakAuthenticationExecutionConfig(flowAlias, configAlias, configProvider string, priority int) string { return fmt.Sprintf(` data "keycloak_realm" "realm" { realm = "%s" @@ -199,6 +199,7 @@ resource "keycloak_authentication_execution" "execution" { realm_id = data.keycloak_realm.realm.id parent_flow_alias = keycloak_authentication_flow.flow.alias authenticator = "identity-provider-redirector" + priority = %d } resource "keycloak_authentication_execution_config" "config" { @@ -208,5 +209,5 @@ resource "keycloak_authentication_execution_config" "config" { config = { defaultProvider = "%s" } -}`, testAccRealm.Realm, flowAlias, configAlias, configProvider) +}`, testAccRealm.Realm, flowAlias, priority, configAlias, configProvider) } diff --git a/provider/resource_keycloak_authentication_execution_test.go b/provider/resource_keycloak_authentication_execution_test.go index d892a2608..510cddf27 100644 --- a/provider/resource_keycloak_authentication_execution_test.go +++ b/provider/resource_keycloak_authentication_execution_test.go @@ -99,6 +99,41 @@ func TestAccKeycloakAuthenticationExecution_updateAuthenticationExecutionRequire }) } +func TestAccKeycloakAuthenticationExecution_updateAuthenticationExecutionPriority(t *testing.T) { + skipIfVersionIsLessThan(testCtx, t, keycloakClient, keycloak.Version_25) + t.Parallel() + authParentFlowAlias := acctest.RandomWithPrefix("tf-acc") + + resource.Test(t, resource.TestCase{ + ProviderFactories: testAccProviderFactories, + PreCheck: func() { testAccPreCheck(t) }, + CheckDestroy: testAccCheckKeycloakAuthenticationSubFlowDestroy(), + Steps: []resource.TestStep{ + { + Config: testKeycloakAuthenticationExecution_basicWithPriority(authParentFlowAlias, 50), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakAuthenticationExecutionExists("keycloak_authentication_execution.execution"), + resource.TestCheckResourceAttr("keycloak_authentication_execution.execution", "priority", "50"), + ), + }, + { + Config: testKeycloakAuthenticationExecution_basicWithPriority(authParentFlowAlias, 60), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakAuthenticationExecutionExists("keycloak_authentication_execution.execution"), + resource.TestCheckResourceAttr("keycloak_authentication_execution.execution", "priority", "60"), + ), + }, + { + Config: testKeycloakAuthenticationExecution_basicWithPriority(authParentFlowAlias, 70), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakAuthenticationExecutionExists("keycloak_authentication_execution.execution"), + resource.TestCheckResourceAttr("keycloak_authentication_execution.execution", "priority", "70"), + ), + }, + }, + }) +} + func testAccCheckKeycloakAuthenticationExecutionExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { _, err := getAuthenticationExecutionFromState(s, resourceName) @@ -199,7 +234,27 @@ resource "keycloak_authentication_execution" "execution" { `, testAccRealm.Realm, parentAlias) } -func testKeycloakAuthenticationExecution_basicWithRequirement(parentAlias, requirement string) string { +func testKeycloakAuthenticationExecution_basicWithPriority(parentAlias string, priority int) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} + +resource "keycloak_authentication_flow" "flow" { + realm_id = data.keycloak_realm.realm.id + alias = "%s" +} + +resource "keycloak_authentication_execution" "execution" { + realm_id = data.keycloak_realm.realm.id + parent_flow_alias = keycloak_authentication_flow.flow.alias + authenticator = "auth-cookie" + priority = %d +} + `, testAccRealm.Realm, parentAlias, priority) +} + +func testKeycloakAuthenticationExecution_basicWithRequirement(parentAlias string, requirement string) string { return fmt.Sprintf(` data "keycloak_realm" "realm" { realm = "%s" diff --git a/provider/resource_keycloak_authentication_subflow_test.go b/provider/resource_keycloak_authentication_subflow_test.go index f4b25826b..afbdf3fd9 100644 --- a/provider/resource_keycloak_authentication_subflow_test.go +++ b/provider/resource_keycloak_authentication_subflow_test.go @@ -134,6 +134,43 @@ func TestAccKeycloakAuthenticationSubFlow_updateAuthenticationSubFlowRequirement }) } +func TestAccKeycloakAuthenticationSubFlow_updateAuthenticationSubFlowPriority(t *testing.T) { + skipIfVersionIsLessThan(testCtx, t, keycloakClient, keycloak.Version_25) + t.Parallel() + + authParentFlowAlias := acctest.RandomWithPrefix("tf-acc") + authFlowAlias := acctest.RandomWithPrefix("tf-acc") + + resource.Test(t, resource.TestCase{ + ProviderFactories: testAccProviderFactories, + PreCheck: func() { testAccPreCheck(t) }, + CheckDestroy: testAccCheckKeycloakAuthenticationSubFlowDestroy(), + Steps: []resource.TestStep{ + { + Config: testKeycloakAuthenticationSubFlow_basicWithPriority(authParentFlowAlias, authFlowAlias, 10), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakAuthenticationSubFlowExists("keycloak_authentication_subflow.subflow"), + resource.TestCheckResourceAttr("keycloak_authentication_subflow.subflow", "priority", "10"), + ), + }, + { + Config: testKeycloakAuthenticationSubFlow_basicWithPriority(authParentFlowAlias, authFlowAlias, 20), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakAuthenticationSubFlowExists("keycloak_authentication_subflow.subflow"), + resource.TestCheckResourceAttr("keycloak_authentication_subflow.subflow", "priority", "20"), + ), + }, + { + Config: testKeycloakAuthenticationSubFlow_basicWithPriority(authParentFlowAlias, authFlowAlias, 30), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakAuthenticationSubFlowExists("keycloak_authentication_subflow.subflow"), + resource.TestCheckResourceAttr("keycloak_authentication_subflow.subflow", "priority", "30"), + ), + }, + }, + }) +} + func testAccCheckKeycloakAuthenticationSubFlowExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { _, err := getAuthenticationSubFlowFromState(s, resourceName) @@ -237,6 +274,28 @@ resource "keycloak_authentication_subflow" "subflow" { `, testAccRealm.Realm, parentAlias, alias) } +func testKeycloakAuthenticationSubFlow_basicWithPriority(parentAlias, alias string, priority int) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} + +resource "keycloak_authentication_flow" "flow" { + realm_id = data.keycloak_realm.realm.id + alias = "%s" +} + +resource "keycloak_authentication_subflow" "subflow" { + realm_id = data.keycloak_realm.realm.id + parent_flow_alias = keycloak_authentication_flow.flow.alias + priority = %d + + alias = "%s" + provider_id = "basic-flow" +} + `, testAccRealm.Realm, parentAlias, priority, alias) +} + func testKeycloakAuthenticationSubFlow_basicWithRequirement(parentAlias, alias, requirement string) string { return fmt.Sprintf(` data "keycloak_realm" "realm" { diff --git a/provider/test_utils.go b/provider/test_utils.go index cc93939b9..ee023e3ba 100644 --- a/provider/test_utils.go +++ b/provider/test_utils.go @@ -79,6 +79,17 @@ func skipIfVersionIsLessThanOrEqualTo(ctx context.Context, t *testing.T, keycloa } } +func skipIfVersionIsLessThan(ctx context.Context, t *testing.T, keycloakClient *keycloak.KeycloakClient, version keycloak.Version) { + ok, err := keycloakClient.VersionIsLessThan(ctx, version) + if err != nil { + t.Errorf("error checking keycloak version: %v", err) + } + + if ok { + t.Skipf("keycloak server version is less than %s, skipping...", version) + } +} + func skipIfVersionIsGreaterThanOrEqualTo(ctx context.Context, t *testing.T, keycloakClient *keycloak.KeycloakClient, version keycloak.Version) { ok, err := keycloakClient.VersionIsGreaterThanOrEqualTo(ctx, version) if err != nil {