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.

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

AzureMonitor: Deprecate using separate credentials for Azure Monitor Logs #34758

Merged
merged 3 commits into from May 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,5 +1,5 @@
import React from 'react';
import { shallow } from 'enzyme';
import { render, screen } from '@testing-library/react';
import AnalyticsConfig, { Props } from './AnalyticsConfig';

const setup = (propsFunc?: (props: Props) => Props) => {
Expand Down Expand Up @@ -47,14 +47,14 @@ const setup = (propsFunc?: (props: Props) => Props) => {
props = propsFunc(props);
}

return shallow(<AnalyticsConfig {...props} />);
return render(<AnalyticsConfig {...props} />);
};

describe('Render', () => {
it('should render component', () => {
const wrapper = setup();

expect(wrapper).toMatchSnapshot();
expect(wrapper.baseElement).toMatchSnapshot();
});

it('should disable log analytics credentials form', () => {
Expand All @@ -68,7 +68,7 @@ describe('Render', () => {
},
},
}));
expect(wrapper).toMatchSnapshot();
expect(wrapper.baseElement).toMatchSnapshot();
});

it('should enable azure log analytics load workspaces button', () => {
Expand All @@ -87,6 +87,37 @@ describe('Render', () => {
},
},
}));
expect(wrapper).toMatchSnapshot();
expect(wrapper.baseElement).toMatchSnapshot();
});

it('should not render the Switch to use different creds for log analytics by default', () => {
setup((props) => ({
...props,
options: {
...props.options,
jsonData: {
...props.options.jsonData,
azureLogAnalyticsSameAs: undefined,
},
},
}));
expect(screen.queryByLabelText('Same details as Azure Monitor API')).not.toBeInTheDocument();
expect(screen.queryByText('is deprecated', { exact: false })).not.toBeInTheDocument();
});

// Remove this test with deprecated code
it('should not render the Switch if different creds for log analytics were set from before', () => {
setup((props) => ({
...props,
options: {
...props.options,
jsonData: {
...props.options.jsonData,
azureLogAnalyticsSameAs: false,
},
},
}));
expect(screen.queryByLabelText('Same details as Azure Monitor API')).toBeInTheDocument();
expect(screen.queryByText('is deprecated', { exact: false })).toBeInTheDocument();
});
});
@@ -1,7 +1,7 @@
import React, { FunctionComponent, useEffect, useMemo, useReducer, useState } from 'react';
import { SelectableValue } from '@grafana/data';
import { AzureCredentialsForm } from './AzureCredentialsForm';
import { InlineFormLabel, LegacyForms, Button } from '@grafana/ui';
import { InlineFormLabel, LegacyForms, Button, Alert } from '@grafana/ui';
const { Select, Switch } = LegacyForms;
import { AzureDataSourceSettings, AzureCredentials } from '../types';
import {
Expand All @@ -23,11 +23,15 @@ export const AnalyticsConfig: FunctionComponent<Props> = (props: Props) => {
const { updateOptions, getSubscriptions, getWorkspaces } = props;
const primaryCredentials = useMemo(() => getCredentials(props.options), [props.options]);
const logAnalyticsCredentials = useMemo(() => getLogAnalyticsCredentials(props.options), [props.options]);

const subscriptionId = logAnalyticsCredentials
? logAnalyticsCredentials.defaultSubscriptionId
: primaryCredentials.defaultSubscriptionId;

const credentialsEnabled = primaryCredentials.authType === 'clientsecret';
// Only show a section for setting LogAnalytics credentials if they were set from before
// And the authType is supported
const [credentialsUsed, _] = useState(!!logAnalyticsCredentials);
const credentialsEnabled = credentialsUsed && primaryCredentials.authType === 'clientsecret';

const hasRequiredFields =
subscriptionId &&
Expand Down Expand Up @@ -134,20 +138,31 @@ export const AnalyticsConfig: FunctionComponent<Props> = (props: Props) => {
onChange={onLogAnalyticsSameAsChange}
{...tooltipAttribute}
/>

{showSameAsHelpMsg && (
<div className="grafana-info-box m-t-2">
<div className="alert-body">
<p>Re-enter your Azure Monitor Client Secret to use this setting.</p>
</div>
</div>
)}
Comment on lines 142 to 148
Copy link
Contributor

Choose a reason for hiding this comment

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

Include this in the condition as well - we don't want to show this if the switch isn't visible.

It might not in practice, but we want to be extra sure 😅


{logAnalyticsCredentials && (
<AzureCredentialsForm
managedIdentityEnabled={false}
credentials={logAnalyticsCredentials}
onCredentialsChange={onCredentialsChange}
getSubscriptions={getSubscriptions}
/>
<>
<Alert severity="info" title="Deprecated">
Using different credentials for Azure Monitor Logs is deprecated and will be removed in a future
version.
<br />
Create a different Data Source if you need to use different credentials.
</Alert>

<AzureCredentialsForm
managedIdentityEnabled={false}
credentials={logAnalyticsCredentials}
onCredentialsChange={onCredentialsChange}
getSubscriptions={getSubscriptions}
/>
</>
)}
</>
)}
Expand Down