Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up on Azure configuration #29482

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Clean up on Azure configuration #29482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7440b09
put in more options and make generic root password ttl
Monkeychip Feb 3, 2025
077c932
small changes
Monkeychip Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ui/app/components/secret-engine/configure-wif.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export default class ConfigureWif extends Component<Args> {
// reset all "account" attributes that are mutually exclusive with "wif" attributes
// these attributes are different for each engine
type === 'azure'
? (mountConfigModel.clientSecret = mountConfigModel.rootPasswordTtl = undefined)
? (mountConfigModel.clientSecret = undefined)
: type === 'aws'
? (mountConfigModel.accessKey = undefined)
: null;
Expand Down
35 changes: 22 additions & 13 deletions ui/app/models/azure/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ export default class AzureConfig extends Model {
@attr('string', { label: 'Tenant ID' }) tenantId;
@attr('string', { label: 'Client ID' }) clientId;
@attr('string', { sensitive: true }) clientSecret; // obfuscated, never returned by API
@attr('string') environment;

@attr('string', {
subText:
'This value can also be provided with the AZURE_ENVIRONMENT environment variable. If not specified, Vault will use Azure Public Cloud.',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text was in the full Azure build out designs, and it seemed helpful so I added.

})
environment;

@attr('string', {
subText:
Expand All @@ -34,8 +39,10 @@ export default class AzureConfig extends Model {
@attr({
label: 'Root password TTL',
editType: 'ttl',
helperTextDisabled:
'Specifies how long the root password is valid for in Azure when rotate-root generates a new client secret. Defaults to 182 days or 6 months, 1 day and 13 hours.',
// default is 15768000 sec. The api docs say 182 days, but this should be updated to 182.5 days.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to ask the backend about this, but for now I wanted to leave the note in the code because it's confusing when we formate the returned default to 6 months 1 day 13 hours but if you pass in 182 days this is formatted to 6 months 1 day 1 hour.

helperTextDisabled: 'Vault will use the default of 182 days.',
helperTextEnabled:
'Specifies how long the root password is valid for in Azure when rotate-root generates a new client secret.',
})
rootPasswordTtl;

Expand Down Expand Up @@ -64,11 +71,9 @@ export default class AzureConfig extends Model {
return !!this.identityTokenAudience || !!this.identityTokenTtl;
}

get isAccountPluginConfigured() {
// clientSecret is not checked here because it's never return by the API
// however it is an Azure account field
return !!this.rootPasswordTtl;
}
// the "clientSecret" param is not checked because it's never return by the API.
// thus we can never say for sure if the account accessType has been configured so we always return false
isAccountPluginConfigured = false;

/* GETTERS used to generate array of fields to be displayed in:
1. details view
Expand All @@ -91,18 +96,22 @@ export default class AzureConfig extends Model {
formFieldGroups(accessType = 'account') {
const formFieldGroups = [];
formFieldGroups.push({
default: ['subscriptionId', 'tenantId', 'clientId', 'environment'],
default: ['subscriptionId', 'tenantId', 'clientId'],
});
if (accessType === 'wif') {
if (accessType === 'account') {
formFieldGroups.push({
default: ['identityTokenAudience', 'identityTokenTtl'],
default: ['clientSecret'],
});
}
if (accessType === 'account') {
if (accessType === 'wif') {
formFieldGroups.push({
default: ['clientSecret', 'rootPasswordTtl'],
default: ['identityTokenAudience', 'identityTokenTtl'],
});
}
formFieldGroups.push({
'More options': ['environment', 'rootPasswordTtl'],
});

return formFieldGroups;
}
}
2 changes: 1 addition & 1 deletion ui/app/models/gcp/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ export default class GcpConfig extends Model {
return !!this.identityTokenAudience || !!this.identityTokenTtl || !!this.serviceAccountEmail;
}

isAccountPluginConfigured = false;
// the "credentials" param is not checked for "isAccountPluginConfigured" because it's never return by the API
// additionally credentials can be set via GOOGLE_APPLICATION_CREDENTIALS env var so we cannot call it a required field in the ui.
// thus we can never say for sure if the account accessType has been configured so we always return false
isAccountPluginConfigured = false;

get displayAttrs() {
const formFields = expandAttributeMeta(this, this.configurableParams);
Expand Down
25 changes: 13 additions & 12 deletions ui/tests/acceptance/secrets/backend/azure/azure-configuration-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ module('Acceptance | Azure | configuration', function (hooks) {
subscription_id: 'subscription-id',
tenant_id: 'tenant-id',
client_id: 'client-id',
root_password_ttl: '20 days 20 hours',
environment: 'AZUREPUBLICCLOUD',
root_password_ttl: '1800000s',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have passed in seconds originally as that's what the api does.

};
this.server.get(`${path}/config`, () => {
assert.true(true, 'request made to config when navigating to the configuration page.');
Expand All @@ -112,7 +112,8 @@ module('Acceptance | Azure | configuration', function (hooks) {
});

module('create', function () {
test('it should save azure account accessType options', async function (assert) {
test('it should save azure account options', async function (assert) {
// there are no azure specific options that can be returned from the API so confirm the generic options are saved.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very similar acceptance tests that we do for GCP as it too cannot check if GCP specific options where set.

assert.expect(3);
const path = `azure-${this.uid}`;
await enablePage.enable(this.type, path);
Expand All @@ -125,18 +126,15 @@ module('Acceptance | Azure | configuration', function (hooks) {

await click(SES.configTab);
await click(SES.configure);
await fillInAzureConfig(this.type);
await fillInAzureConfig();
await click(GENERAL.saveButton);
assert.true(
this.flashSuccessSpy.calledWith(`Successfully saved ${path}'s configuration.`),
'Success flash message is rendered showing the azure model configuration was saved.'
);
assert
.dom(GENERAL.infoRowValue('Root password TTL'))
.hasText(
'1 hour 26 minutes 40 seconds',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I messed around some with the secret-engine-helper.js method fillInAzureConfig so this value changed.

'Root password TTL, an azure account specific field, has been set.'
);
.hasText('3 minutes 20 seconds', 'Root password TTL, a generic field, has been set.');
assert
.dom(GENERAL.infoRowValue('Subscription ID'))
.hasText('subscription-id', 'Subscription ID, a generic field, has been set.');
Expand Down Expand Up @@ -226,7 +224,7 @@ module('Acceptance | Azure | configuration', function (hooks) {

await click(SES.configTab);
await click(SES.configure);
await fillInAzureConfig('azure');
await fillInAzureConfig();
await click(GENERAL.saveButton);

assert.dom(GENERAL.messageError).hasText('Error welp, that did not work!', 'API error shows on form');
Expand Down Expand Up @@ -267,12 +265,15 @@ module('Acceptance | Azure | configuration', function (hooks) {
identity_token_audience: 'audience',
identity_token_ttl: 720000,
environment: 'AZUREPUBLICCLOUD',
root_password_ttl: '1800000s',
};
this.server.get(`${path}/config`, () => {
assert.true(true, 'request made to config when navigating to the configuration page.');
return { data: { id: path, type: this.type, ...wifAttrs } };
});
await enablePage.enable(this.type, path);
GENERAL.toggleGroup('More options');

for (const key of expectedConfigKeys('azure-wif')) {
const responseKeyAndValue = expectedValueOfConfigKeys(this.type, key);
assert
Expand Down Expand Up @@ -362,7 +363,7 @@ module('Acceptance | Azure | configuration', function (hooks) {

await click(SES.configTab);
await click(SES.configure);
await fillInAzureConfig('withWif');
await fillInAzureConfig(true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed this method to match the feedback on similar method with fillInGcpConfig see here.

await click(GENERAL.saveButton);
assert.dom(SES.wif.issuerWarningModal).doesNotExist('issuer warning modal does not show');
assert.true(
Expand Down Expand Up @@ -392,7 +393,7 @@ module('Acceptance | Azure | configuration', function (hooks) {

await click(SES.configTab);
await click(SES.configure);
await fillInAzureConfig('withWif');
await fillInAzureConfig(true);
assert
.dom(GENERAL.inputByAttr('issuer'))
.hasValue(oldIssuer, 'issuer defaults to previously saved value');
Expand Down Expand Up @@ -434,7 +435,7 @@ module('Acceptance | Azure | configuration', function (hooks) {

await click(SES.configTab);
await click(SES.configure);
await fillInAzureConfig('withWif');
await fillInAzureConfig(true);
assert
.dom(GENERAL.inputByAttr('issuer'))
.hasValue(oldIssuer, 'issuer defaults to previously saved value');
Expand Down Expand Up @@ -465,7 +466,7 @@ module('Acceptance | Azure | configuration', function (hooks) {
await enablePage.enable(this.type, path);
await click(SES.configTab);
await click(SES.configure);
await fillInAzureConfig('withWif');
await fillInAzureConfig(true);
await click(GENERAL.saveButton); // finished creating attributes, go back and edit them.
assert
.dom(GENERAL.infoRowValue('Identity token audience'))
Expand Down
32 changes: 16 additions & 16 deletions ui/tests/helpers/secret-engine/secret-engine-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ const createAzureConfig = (store, backend, accessType = 'generic') => {
subscription_id: 'subscription-id',
tenant_id: 'tenant-id',
client_id: 'client-id',
root_password_ttl: '20 days 20 hours',
root_password_ttl: '1800000s',
environment: 'AZUREPUBLICCLOUD',
},
});
Expand All @@ -144,7 +144,7 @@ const createAzureConfig = (store, backend, accessType = 'generic') => {
client_id: 'client-id',
identity_token_audience: 'audience',
identity_token_ttl: 7200,
root_password_ttl: '20 days 20 hours',
root_password_ttl: '1800000s',
environment: 'AZUREPUBLICCLOUD',
},
});
Expand All @@ -158,6 +158,7 @@ const createAzureConfig = (store, backend, accessType = 'generic') => {
tenant_id: 'tenant-id-2',
client_id: 'client-id-2',
environment: 'AZUREPUBLICCLOUD',
root_password_ttl: '1800000s',
},
});
}
Expand Down Expand Up @@ -250,22 +251,21 @@ export const fillInAwsConfig = async (situation = 'withAccess') => {
}
};

export const fillInAzureConfig = async (situation = 'azure') => {
await fillIn(GENERAL.inputByAttr('subscriptionId'), 'subscription-id');
await fillIn(GENERAL.inputByAttr('tenantId'), 'tenant-id');
await fillIn(GENERAL.inputByAttr('clientId'), 'client-id');
await fillIn(GENERAL.inputByAttr('environment'), 'AZUREPUBLICCLOUD');

if (situation === 'azure') {
await fillIn(GENERAL.inputByAttr('clientSecret'), 'client-secret');
await click(GENERAL.ttl.toggle('Root password TTL'));
await fillIn(GENERAL.ttl.input('Root password TTL'), '5200');
}
if (situation === 'withWif') {
export const fillInAzureConfig = async (withWif = false) => {
if (withWif) {
await click(SES.wif.accessType('wif')); // toggle to wif
await fillIn(GENERAL.inputByAttr('identityTokenAudience'), 'azure-audience');
await click(GENERAL.ttl.toggle('Identity token TTL'));
await fillIn(GENERAL.ttl.input('Identity token TTL'), '7200');
} else {
await fillIn(GENERAL.inputByAttr('subscriptionId'), 'subscription-id');
await fillIn(GENERAL.inputByAttr('tenantId'), 'tenant-id');
await fillIn(GENERAL.inputByAttr('clientId'), 'client-id');
await click(GENERAL.toggleGroup('More options'));
await fillIn(GENERAL.inputByAttr('environment'), 'AZUREPUBLICCLOUD');
await click(GENERAL.ttl.toggle('Root password TTL'));
await fillIn(GENERAL.ttl.input('Root password TTL'), '200');
await fillIn(GENERAL.inputByAttr('clientSecret'), 'client-secret');
}
};

Expand Down Expand Up @@ -298,8 +298,8 @@ const awsLeaseKeys = ['Default Lease TTL', 'Max Lease TTL'];
const awsKeys = ['Access key', 'Secret key', 'Region', 'IAM endpoint', 'STS endpoint', 'Max retries'];
const awsWifKeys = ['Issuer', 'Role ARN', ...genericWifKeys];
// Azure specific keys
const genericAzureKeys = ['Subscription ID', 'Tenant ID', 'Client ID', 'Environment'];
const azureKeys = [...genericAzureKeys, 'Client secret', 'Root password TTL'];
const genericAzureKeys = ['Subscription ID', 'Tenant ID', 'Client ID', 'Environment', 'Root password TTL'];
const azureKeys = [...genericAzureKeys, 'Client secret'];
const azureWifKeys = [...genericAzureKeys, ...genericWifKeys];
// GCP specific keys
const genericGcpKeys = ['Config TTL', 'Max TTL'];
Expand Down
Loading
Loading