Skip to content

Commit c2152fb

Browse files
julienduchesnejulienduchesne
authored
Ease of use: Add validation that org_id isn't used with API Keys/SA Tokens (#1296)
Closes #1157 A source of confusion that we get occasionally get questions about is when users try to create org resources with an API key API keys are org-scoped already and when they are used, the org header will be ignored by the API This means that the resources will still be created but not where the user expects This PR adds a validation on the Create function of every resource. If org_id is set and we're using an API key, then throw an error instead of the unexpected behavior
1 parent 44f5cda commit c2152fb

File tree

4 files changed

+156
-63
lines changed

4 files changed

+156
-63
lines changed

‎internal/provider/configure_clients.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ func createGrafanaOAPIClient(client *common.Client, providerConfig frameworkProv
8585
return err
8686
}
8787

88+
if orgID > 1 && apiKey != "" {
89+
return fmt.Errorf("org_id is only supported with basic auth. API keys are already org-scoped")
90+
}
91+
8892
cfg := goapi.TransportConfig{
8993
Host: client.GrafanaAPIURLParsed.Host,
9094
BasePath: apiPath,

‎internal/provider/legacy_provider.go‎

Lines changed: 62 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -36,44 +36,47 @@ func init() {
3636
func Provider(version string) *schema.Provider {
3737
var (
3838
// Resources that require the Grafana client to exist.
39-
grafanaClientResources = addResourcesMetadataValidation(grafanaClientPresent, map[string]*schema.Resource{
40-
// Grafana
41-
"grafana_annotation": grafana.ResourceAnnotation(),
42-
"grafana_api_key": grafana.ResourceAPIKey(),
43-
"grafana_contact_point": grafana.ResourceContactPoint(),
44-
"grafana_dashboard": grafana.ResourceDashboard(),
45-
"grafana_dashboard_public": grafana.ResourcePublicDashboard(),
46-
"grafana_dashboard_permission": grafana.ResourceDashboardPermission(),
47-
"grafana_data_source": grafana.ResourceDataSource(),
48-
"grafana_data_source_permission": grafana.ResourceDatasourcePermission(),
49-
"grafana_folder": grafana.ResourceFolder(),
50-
"grafana_folder_permission": grafana.ResourceFolderPermission(),
51-
"grafana_library_panel": grafana.ResourceLibraryPanel(),
52-
"grafana_message_template": grafana.ResourceMessageTemplate(),
53-
"grafana_mute_timing": grafana.ResourceMuteTiming(),
54-
"grafana_notification_policy": grafana.ResourceNotificationPolicy(),
55-
"grafana_organization": grafana.ResourceOrganization(),
56-
"grafana_organization_preferences": grafana.ResourceOrganizationPreferences(),
57-
"grafana_playlist": grafana.ResourcePlaylist(),
58-
"grafana_report": grafana.ResourceReport(),
59-
"grafana_role": grafana.ResourceRole(),
60-
"grafana_role_assignment": grafana.ResourceRoleAssignment(),
61-
"grafana_rule_group": grafana.ResourceRuleGroup(),
62-
"grafana_team": grafana.ResourceTeam(),
63-
"grafana_team_external_group": grafana.ResourceTeamExternalGroup(),
64-
"grafana_service_account_token": grafana.ResourceServiceAccountToken(),
65-
"grafana_service_account": grafana.ResourceServiceAccount(),
66-
"grafana_service_account_permission": grafana.ResourceServiceAccountPermission(),
67-
"grafana_user": grafana.ResourceUser(),
68-
69-
// Machine Learning
70-
"grafana_machine_learning_job": machinelearning.ResourceJob(),
71-
"grafana_machine_learning_holiday": machinelearning.ResourceHoliday(),
72-
"grafana_machine_learning_outlier_detector": machinelearning.ResourceOutlierDetector(),
73-
74-
// SLO
75-
"grafana_slo": slo.ResourceSlo(),
76-
})
39+
grafanaClientResources = addCreateReadResourcesMetadataValidation(
40+
readGrafanaClientValidation,
41+
createGrafanaClientValidation,
42+
map[string]*schema.Resource{
43+
// Grafana
44+
"grafana_annotation": grafana.ResourceAnnotation(),
45+
"grafana_api_key": grafana.ResourceAPIKey(),
46+
"grafana_contact_point": grafana.ResourceContactPoint(),
47+
"grafana_dashboard": grafana.ResourceDashboard(),
48+
"grafana_dashboard_public": grafana.ResourcePublicDashboard(),
49+
"grafana_dashboard_permission": grafana.ResourceDashboardPermission(),
50+
"grafana_data_source": grafana.ResourceDataSource(),
51+
"grafana_data_source_permission": grafana.ResourceDatasourcePermission(),
52+
"grafana_folder": grafana.ResourceFolder(),
53+
"grafana_folder_permission": grafana.ResourceFolderPermission(),
54+
"grafana_library_panel": grafana.ResourceLibraryPanel(),
55+
"grafana_message_template": grafana.ResourceMessageTemplate(),
56+
"grafana_mute_timing": grafana.ResourceMuteTiming(),
57+
"grafana_notification_policy": grafana.ResourceNotificationPolicy(),
58+
"grafana_organization": grafana.ResourceOrganization(),
59+
"grafana_organization_preferences": grafana.ResourceOrganizationPreferences(),
60+
"grafana_playlist": grafana.ResourcePlaylist(),
61+
"grafana_report": grafana.ResourceReport(),
62+
"grafana_role": grafana.ResourceRole(),
63+
"grafana_role_assignment": grafana.ResourceRoleAssignment(),
64+
"grafana_rule_group": grafana.ResourceRuleGroup(),
65+
"grafana_team": grafana.ResourceTeam(),
66+
"grafana_team_external_group": grafana.ResourceTeamExternalGroup(),
67+
"grafana_service_account_token": grafana.ResourceServiceAccountToken(),
68+
"grafana_service_account": grafana.ResourceServiceAccount(),
69+
"grafana_service_account_permission": grafana.ResourceServiceAccountPermission(),
70+
"grafana_user": grafana.ResourceUser(),
71+
72+
// Machine Learning
73+
"grafana_machine_learning_job": machinelearning.ResourceJob(),
74+
"grafana_machine_learning_holiday": machinelearning.ResourceHoliday(),
75+
"grafana_machine_learning_outlier_detector": machinelearning.ResourceOutlierDetector(),
76+
77+
// SLO
78+
"grafana_slo": slo.ResourceSlo(),
79+
})
7780

7881
// Resources that require the Synthetic Monitoring client to exist.
7982
smClientResources = addResourcesMetadataValidation(smClientPresent, map[string]*schema.Resource{
@@ -106,24 +109,27 @@ func Provider(version string) *schema.Provider {
106109
})
107110

108111
// Datasources that require the Grafana client to exist.
109-
grafanaClientDatasources = addResourcesMetadataValidation(grafanaClientPresent, map[string]*schema.Resource{
110-
"grafana_dashboard": grafana.DatasourceDashboard(),
111-
"grafana_dashboards": grafana.DatasourceDashboards(),
112-
"grafana_data_source": grafana.DatasourceDatasource(),
113-
"grafana_folder": grafana.DatasourceFolder(),
114-
"grafana_folders": grafana.DatasourceFolders(),
115-
"grafana_library_panel": grafana.DatasourceLibraryPanel(),
116-
"grafana_user": grafana.DatasourceUser(),
117-
"grafana_users": grafana.DatasourceUsers(),
118-
"grafana_role": grafana.DatasourceRole(),
119-
"grafana_service_account": grafana.DatasourceServiceAccount(),
120-
"grafana_team": grafana.DatasourceTeam(),
121-
"grafana_organization": grafana.DatasourceOrganization(),
122-
"grafana_organization_preferences": grafana.DatasourceOrganizationPreferences(),
123-
124-
// SLO
125-
"grafana_slos": slo.DatasourceSlo(),
126-
})
112+
grafanaClientDatasources = addCreateReadResourcesMetadataValidation(
113+
readGrafanaClientValidation,
114+
createGrafanaClientValidation,
115+
map[string]*schema.Resource{
116+
"grafana_dashboard": grafana.DatasourceDashboard(),
117+
"grafana_dashboards": grafana.DatasourceDashboards(),
118+
"grafana_data_source": grafana.DatasourceDatasource(),
119+
"grafana_folder": grafana.DatasourceFolder(),
120+
"grafana_folders": grafana.DatasourceFolders(),
121+
"grafana_library_panel": grafana.DatasourceLibraryPanel(),
122+
"grafana_user": grafana.DatasourceUser(),
123+
"grafana_users": grafana.DatasourceUsers(),
124+
"grafana_role": grafana.DatasourceRole(),
125+
"grafana_service_account": grafana.DatasourceServiceAccount(),
126+
"grafana_team": grafana.DatasourceTeam(),
127+
"grafana_organization": grafana.DatasourceOrganization(),
128+
"grafana_organization_preferences": grafana.DatasourceOrganizationPreferences(),
129+
130+
// SLO
131+
"grafana_slos": slo.DatasourceSlo(),
132+
})
127133

128134
// Datasources that require the Synthetic Monitoring client to exist.
129135
smClientDatasources = addResourcesMetadataValidation(smClientPresent, map[string]*schema.Resource{

‎internal/provider/legacy_provider_validation.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,37 +14,53 @@ import (
1414
// These validations are added to the Create and Read functions of all resources,
1515
// because they are entrypoints (code that will be run in all cases).
1616

17-
type metadataValidation func(resourceName string, m interface{}) error
17+
type metadataValidation func(resourceName string, d *schema.ResourceData, m interface{}) error
1818

19-
func grafanaClientPresent(resourceName string, m interface{}) error {
19+
func readGrafanaClientValidation(resourceName string, d *schema.ResourceData, m interface{}) error {
2020
if m.(*common.Client).GrafanaOAPI == nil {
2121
return fmt.Errorf("the Grafana client is required for `%s`. Set the auth and url provider attributes", resourceName)
2222
}
2323
return nil
2424
}
2525

26-
func smClientPresent(resourceName string, m interface{}) error {
26+
func createGrafanaClientValidation(resourceName string, d *schema.ResourceData, m interface{}) error {
27+
if err := readGrafanaClientValidation(resourceName, d, m); err != nil {
28+
return err
29+
}
30+
orgID, ok := d.GetOk("org_id")
31+
orgIDStr, orgIDOk := orgID.(string)
32+
if ok && orgIDOk && orgIDStr != "" && orgIDStr != "0" && m.(*common.Client).GrafanaAPIConfig.APIKey != "" {
33+
return fmt.Errorf("org_id is only supported with basic auth. API keys are already org-scoped")
34+
}
35+
return nil
36+
}
37+
38+
func smClientPresent(resourceName string, d *schema.ResourceData, m interface{}) error {
2739
if m.(*common.Client).SMAPI == nil {
2840
return fmt.Errorf("the Synthetic Monitoring client is required for `%s`. Set the sm_access_token provider attribute", resourceName)
2941
}
3042
return nil
3143
}
3244

33-
func cloudClientPresent(resourceName string, m interface{}) error {
45+
func cloudClientPresent(resourceName string, d *schema.ResourceData, m interface{}) error {
3446
if m.(*common.Client).GrafanaCloudAPI == nil {
3547
return fmt.Errorf("the Cloud API client is required for `%s`. Set the cloud_api_key provider attribute", resourceName)
3648
}
3749
return nil
3850
}
3951

40-
func onCallClientPresent(resourceName string, m interface{}) error {
52+
func onCallClientPresent(resourceName string, d *schema.ResourceData, m interface{}) error {
4153
if m.(*common.Client).OnCallClient == nil {
4254
return fmt.Errorf("the Oncall client is required for `%s`. Set the oncall_access_token provider attribute", resourceName)
4355
}
4456
return nil
4557
}
4658

4759
func addResourcesMetadataValidation(validateFunc metadataValidation, resources map[string]*schema.Resource) map[string]*schema.Resource {
60+
return addCreateReadResourcesMetadataValidation(validateFunc, validateFunc, resources)
61+
}
62+
63+
func addCreateReadResourcesMetadataValidation(readValidateFunc, createValidateFunc metadataValidation, resources map[string]*schema.Resource) map[string]*schema.Resource {
4864
for name, r := range resources {
4965
name := name
5066
//nolint:staticcheck
@@ -54,7 +70,7 @@ func addResourcesMetadataValidation(validateFunc metadataValidation, resources m
5470
if r.ReadContext != nil {
5571
prev := r.ReadContext
5672
r.ReadContext = func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
57-
if err := validateFunc(name, m); err != nil {
73+
if err := readValidateFunc(name, d, m); err != nil {
5874
return diag.FromErr(err)
5975
}
6076
return prev(ctx, d, m)
@@ -67,7 +83,7 @@ func addResourcesMetadataValidation(validateFunc metadataValidation, resources m
6783
if r.CreateContext != nil {
6884
prev := r.CreateContext
6985
r.CreateContext = func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
70-
if err := validateFunc(name, m); err != nil {
86+
if err := createValidateFunc(name, d, m); err != nil {
7187
return diag.FromErr(err)
7288
}
7389
return prev(ctx, d, m)

‎internal/resources/grafana/resource_team_test.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@ package grafana_test
22

33
import (
44
"fmt"
5+
"os"
6+
"regexp"
7+
"strconv"
58
"strings"
69
"testing"
710

11+
"github.com/grafana/grafana-openapi-client-go/client/service_accounts"
812
"github.com/grafana/grafana-openapi-client-go/models"
913
"github.com/grafana/terraform-provider-grafana/internal/common"
1014
"github.com/grafana/terraform-provider-grafana/internal/resources/grafana"
@@ -317,6 +321,69 @@ func TestAccResourceTeam_InOrg(t *testing.T) {
317321
})
318322
}
319323

324+
// This tests that API keys/service account tokens cannot be used at the same time as org_id
325+
// because API keys are already org-scoped.
326+
func TestAccTeam_OrgScopedOnAPIKey(t *testing.T) {
327+
testutils.CheckOSSTestsEnabled(t, ">=9.1.0")
328+
329+
// Create a service account within an org
330+
name := acctest.RandString(10)
331+
globalClient := grafana.OAPIGlobalClient(testutils.Provider.Meta())
332+
org, err := globalClient.Orgs.CreateOrg(&models.CreateOrgCommand{Name: name})
333+
if err != nil {
334+
t.Fatal(err)
335+
}
336+
defer func() {
337+
if _, err := globalClient.Orgs.DeleteOrgByID(*org.Payload.OrgID); err != nil {
338+
t.Fatal(err)
339+
}
340+
}()
341+
orgClient := grafana.OAPIGlobalClient(testutils.Provider.Meta()).WithOrgID(*org.Payload.OrgID)
342+
sa, err := orgClient.ServiceAccounts.CreateServiceAccount(
343+
service_accounts.NewCreateServiceAccountParams().WithBody(&models.CreateServiceAccountForm{
344+
Name: name,
345+
Role: "Admin",
346+
},
347+
))
348+
if err != nil {
349+
t.Fatal(err)
350+
}
351+
saToken, err := orgClient.ServiceAccounts.CreateToken(
352+
service_accounts.NewCreateTokenParams().WithBody(&models.AddServiceAccountTokenCommand{
353+
Name: name,
354+
},
355+
).WithServiceAccountID(sa.Payload.ID),
356+
)
357+
if err != nil {
358+
t.Fatal(err)
359+
}
360+
361+
prevAuth := os.Getenv("GRAFANA_AUTH")
362+
os.Setenv("GRAFANA_AUTH", saToken.Payload.Key)
363+
defer os.Setenv("GRAFANA_AUTH", prevAuth)
364+
resource.Test(t, resource.TestCase{
365+
ProviderFactories: testutils.ProviderFactories,
366+
Steps: []resource.TestStep{
367+
{
368+
Config: fmt.Sprintf(`resource "grafana_team" "test" {
369+
org_id = %d
370+
name = "test"
371+
}`, *org.Payload.OrgID),
372+
ExpectError: regexp.MustCompile("org_id is only supported with basic auth. API keys are already org-scoped"),
373+
},
374+
{
375+
Config: `resource "grafana_team" "test" {
376+
name = "test"
377+
}`,
378+
Check: resource.ComposeTestCheckFunc(
379+
resource.TestCheckResourceAttr("grafana_team.test", "name", "test"),
380+
resource.TestCheckResourceAttr("grafana_team.test", "org_id", strconv.FormatInt(*org.Payload.OrgID, 10)),
381+
),
382+
},
383+
},
384+
})
385+
}
386+
320387
func testAccTeamDefinition(name string, teamMembers []string, withPreferences bool, externalGroups []string) string {
321388
withPreferencesBlock := ""
322389
if withPreferences {

0 commit comments

Comments
 (0)