Skip to content

Commit

Permalink
Remove the use_msi setting from the azure_msi plugin (spiffe#5209)
Browse files Browse the repository at this point in the history
Signed-off-by: Agustín Martínez Fayó <[email protected]>
  • Loading branch information
amartinezfayo authored Jun 10, 2024
1 parent 18767a7 commit 31dbc47
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 43 deletions.
2 changes: 0 additions & 2 deletions doc/plugin_server_nodeattestor_azure_msi.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ Each tenant in the main configuration supports the following
| Configuration | Required | Description | Default |
|-------------------|--------------------------------------|-----------------------------------------------------------------------------------------------------------|---------------------------------|
| `resource_id` | Optional | The resource ID (or audience) for the tenant's MSI token. Tokens for a different resource ID are rejected | <https://management.azure.com/> |
| `use_msi` | [Deprecated](#authenticating-to-azure) | Whether or not to use MSI to authenticate to Azure services for selector resolution. | false |
| `subscription_id` | [Optional](#authenticating-to-azure) | The subscription the tenant resides in | |
| `app_id` | [Optional](#authenticating-to-azure) | The application id | |
| `app_secret` | [Optional](#authenticating-to-azure) | The application secret | |
Expand Down Expand Up @@ -81,7 +80,6 @@ required, however, it will be in a future release.
tenants = {
"00000000-1111-2222-3333-444444444444" = {
resource_id = "http://example.org/app/"
use_msi = true
}
}
}
Expand Down
11 changes: 0 additions & 11 deletions pkg/server/plugin/nodeattestor/azuremsi/msi.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ type TenantConfig struct {
SubscriptionID string `hcl:"subscription_id" json:"subscription_id"`
AppID string `hcl:"app_id" json:"app_id"`
AppSecret string `hcl:"app_secret" json:"app_secret"`

// Deprecated: use_msi is deprecated and will be removed in a future release.
// Will be used implicitly if other mechanisms to authenticate fail.
UseMSI bool `hcl:"use_msi" json:"use_msi"`
}

type MSIAttestorConfig struct {
Expand Down Expand Up @@ -276,9 +272,6 @@ func (p *MSIAttestorPlugin) Configure(_ context.Context, req *configv1.Configure
// Use tenant-specific credentials for resolving selectors
switch {
case tenant.SubscriptionID != "", tenant.AppID != "", tenant.AppSecret != "":
if tenant.UseMSI {
return nil, status.Errorf(codes.InvalidArgument, "misconfigured tenant %q: cannot use both MSI and app authentication", tenantID)
}
if tenant.SubscriptionID == "" {
return nil, status.Errorf(codes.InvalidArgument, "misconfigured tenant %q: missing subscription id", tenantID)
}
Expand All @@ -299,10 +292,6 @@ func (p *MSIAttestorPlugin) Configure(_ context.Context, req *configv1.Configure
return nil, status.Errorf(codes.Internal, "unable to create client for tenant %q: %v", tenantID, err)
}

case tenant.UseMSI:
p.log.Warn("use_msi is deprecated and will be removed in a future release")
fallthrough // use default credential which attempts to fetch credentials using MSI

default:
instanceMetadata, err := p.hooks.fetchInstanceMetadata(http.DefaultClient)
if err != nil {
Expand Down
33 changes: 3 additions & 30 deletions pkg/server/plugin/nodeattestor/azuremsi/msi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,8 @@ func (s *MSIAttestorSuite) TestAttestSuccessWithCustomSPIFFEIDTemplate() {
tenants = {
"TENANTID" = {
resource_id = "https://example.org/app/"
use_msi = true
}
"TENANTID2" = {
use_msi = true
}
"TENANTID2" = { }
}
agent_path_template = "/{{ .PluginName }}/{{ .TenantID }}"
`)
Expand Down Expand Up @@ -484,7 +481,6 @@ func (s *MSIAttestorSuite) TestConfigure() {
tenants = {
"TENANTID" = {
resource_id = "https://example.org/app/"
use_msi = true
}
}
`, nil)
Expand Down Expand Up @@ -516,33 +512,13 @@ func (s *MSIAttestorSuite) TestConfigure() {
app_id = "APPID"
app_secret = "APPSECRET"
}
"TENANTID2" = {
use_msi = true
}
"TENANTID2" = { }
}
`, nil)
require.NoError(t, err)
require.ElementsMatch(t, []string{"TENANTSUBSCRIPTIONID", "SUBSCRIPTIONID"}, clients)
})

s.T().Run("failure with both app creds and msi", func(t *testing.T) {
err := doConfig(t, coreConfig, `
tenants = {
"TENANTID" = {
resource_id = "https://example.org/app/"
use_msi = true
subscription_id = "TENANTSUBSCRIPTIONID"
app_id = "APPID"
app_secret = "APPSECRET"
}
"TENANTID2" = {
use_msi = true
}
}
`, nil)
spiretest.RequireGRPCStatusContains(t, err, codes.InvalidArgument, `misconfigured tenant "TENANTID": cannot use both MSI and app authentication`)
})

s.T().Run("failure with tenant missing subscription id", func(t *testing.T) {
err := doConfig(t, coreConfig, `
tenants = {
Expand Down Expand Up @@ -666,11 +642,8 @@ func (s *MSIAttestorSuite) loadPlugin(options ...plugintest.Option) nodeattestor
tenants = {
"TENANTID" = {
resource_id = "https://example.org/app/"
use_msi = true
}
"TENANTID2" = {
use_msi = true
}
"TENANTID2" = { }
}
`, options...)
}
Expand Down

0 comments on commit 31dbc47

Please sign in to comment.