Merge branch 'master' into blam/deprecate-v1alpha1

2021-05-20 12:08:35 +02:00
parent 3117288b7b f7973f9d0e
commit 31bf41eb65
44 changed files with 669 additions and 80 deletions
@@ -21,6 +21,7 @@ import {
  GroupEntity,
  ResourceEntity,
  SystemEntity,
+  TemplateEntity,
  UserEntity,
 } from '@backstage/catalog-model';
 import { BuiltinKindsEntityProcessor } from './BuiltinKindsEntityProcessor';
@@ -520,5 +521,47 @@ describe('BuiltinKindsEntityProcessor', () => {
        },
      });
    });
+    it('generates relations for template entities', async () => {
+      const entity: TemplateEntity = {
+        apiVersion: 'backstage.io/v1alpha1',
+        kind: 'Template',
+        metadata: { name: 'n' },
+        spec: {
+          schema: {
+            properties: {
+              description: {
+                title: 'd',
+                type: 'string',
+                description: 'des',
+              },
+            },
+          },
+          templater: 'cookiecutter',
+          path: '.',
+          type: 'service',
+          owner: 'o',
+        },
+      };
+
+      await processor.postProcessEntity(entity, location, emit);
+
+      expect(emit).toBeCalledTimes(2);
+      expect(emit).toBeCalledWith({
+        type: 'relation',
+        relation: {
+          source: { kind: 'Group', namespace: 'default', name: 'o' },
+          type: 'ownerOf',
+          target: { kind: 'Template', namespace: 'default', name: 'n' },
+        },
+      });
+      expect(emit).toBeCalledWith({
+        type: 'relation',
+        relation: {
+          source: { kind: 'Template', namespace: 'default', name: 'n' },
+          type: 'ownedBy',
+          target: { kind: 'Group', namespace: 'default', name: 'o' },
+        },
+      });
+    });
  });
 });
@@ -46,6 +46,7 @@ import {
  resourceEntityV1alpha1Validator,
  SystemEntity,
  systemEntityV1alpha1Validator,
+  TemplateEntity,
  templateEntityV1alpha1Validator,
  templateEntityV1beta2Validator,
  UserEntity,
@@ -131,6 +132,19 @@ export class BuiltinKindsEntityProcessor implements CatalogProcessor {
      }
    }

+    /*
+     * Emit relations for the Template kind
+     */
+    if (entity.kind === 'Template') {
+      const template = entity as TemplateEntity;
+      doEmit(
+        template.spec.owner,
+        { defaultKind: 'Group', defaultNamespace: selfRef.namespace },
+        RELATION_OWNED_BY,
+        RELATION_OWNER_OF,
+      );
+    }
+
    /*
     * Emit relations for the Component kind
     */
@@ -69,7 +69,11 @@ export class LdapOrgReaderProcessor implements CatalogProcessor {
    // Be lazy and create the client each time; even though it's pretty
    // inefficient, we usually only do this once per entire refresh loop and
    // don't have to worry about timeouts and reconnects etc.
-    const client = await LdapClient.create(provider.target, provider.bind);
+    const client = await LdapClient.create(
+      this.logger,
+      provider.target,
+      provider.bind,
+    );
    const { users, groups } = await readLdapOrg(
      client,
      provider.users,
@@ -15,6 +15,7 @@
 */

 import ldap, { Client, SearchEntry, SearchOptions } from 'ldapjs';
+import { Logger } from 'winston';
 import { BindConfig } from './config';
 import { errorString } from './util';
 import {
@@ -31,8 +32,20 @@ import {
 export class LdapClient {
  private vendor: Promise<LdapVendor> | undefined;

-  static async create(target: string, bind?: BindConfig): Promise<LdapClient> {
+  static async create(
+    logger: Logger,
+    target: string,
+    bind?: BindConfig,
+  ): Promise<LdapClient> {
    const client = ldap.createClient({ url: target });
+
+    // We want to have a catch-all error handler at the top, since the default
+    // behavior of the client is to blow up the entire process when it fails,
+    // unless an error handler is set.
+    client.on('error', (err: ldap.Error) => {
+      logger.warn(`LDAP client threw an error, ${errorString(err)}`);
+    });
+
    if (!bind) {
      return new LdapClient(client);
    }
@@ -92,7 +105,7 @@ export class LdapClient {
        });
      });
    } catch (e) {
-      throw new Error(`LDAP search at ${dn} failed, ${e.message}`);
+      throw new Error(`LDAP search at DN "${dn}" failed, ${e.message}`);
    }
  }

@@ -105,7 +105,10 @@ describe('readLdapConfig', () => {
              scope: 'base',
              attributes: ['*'],
              filter: 'f',
-              paged: true,
+              paged: {
+                pageSize: 7,
+                pagePause: true,
+              },
            },
            set: { p: 'v' },
            map: {
@@ -153,7 +156,10 @@ describe('readLdapConfig', () => {
            scope: 'base',
            attributes: ['*'],
            filter: 'f',
-            paged: true,
+            paged: {
+              pageSize: 7,
+              pagePause: true,
+            },
          },
          set: { p: 'v' },
          map: {
@@ -180,11 +180,32 @@ export function readLdapConfig(config: Config): LdapProviderConfig[] {
    if (!c) {
      return {};
    }
+
+    const paged = readOptionsPagedConfig(c);
+
    return {
      scope: c.getOptionalString('scope') as SearchOptions['scope'],
      filter: formatFilter(c.getOptionalString('filter')),
      attributes: c.getOptionalStringArray('attributes'),
-      paged: c.getOptionalBoolean('paged'),
+      ...(paged !== undefined ? { paged } : undefined),
+    };
+  }
+
+  function readOptionsPagedConfig(c: Config): SearchOptions['paged'] {
+    const pagedConfig = c.getOptional('paged');
+    if (pagedConfig === undefined) {
+      return undefined;
+    }
+
+    if (pagedConfig === true || pagedConfig === false) {
+      return pagedConfig;
+    }
+
+    const pageSize = c.getOptionalNumber('paged.pageSize');
+    const pagePause = c.getOptionalBoolean('paged.pagePause');
+    return {
+      ...(pageSize !== undefined ? { pageSize } : undefined),
+      ...(pagePause !== undefined ? { pagePause } : undefined),
    };
  }

@@ -258,7 +279,7 @@ export function readLdapConfig(config: Config): LdapProviderConfig[] {
  }

  function formatFilter(filter?: string): string | undefined {
-    // Remove extra whitespaces between blocks to support multiline filters from the configuration
+    // Remove extra whitespace between blocks to support multiline filters from the configuration
    return filter?.replace(/\s*(\(|\))/g, '$1')?.trim();
  }

@@ -108,7 +108,7 @@ export const StepInitAnalyzeUrl = ({
          }
        }
      } catch (e) {
-        setError(e.message);
+        setError(e.data?.error?.message ?? e.message);
        setSubmitted(false);
      }
    },
@@ -25,9 +25,7 @@ yarn add @backstage/plugin-cost-insights

 1. Configure `app-config.yaml`. See [Configuration](#configuration).

-2. Create a CostInsights client. Clients must implement the CostInsightsApi interface. See the [API file](https://github.com/backstage/backstage/blob/master/plugins/cost-insights/src/api/CostInsightsApi.ts) for required methods and documentation.
-
-**Note:** We've briefly explored using the AWS Cost Explorer API to implement a CostInsights client. Learn more about our findings [here](https://github.com/backstage/backstage/blob/master/plugins/cost-insights/contrib/aws-cost-explorer-api.md).
+2. Create a CostInsights client. Clients must implement the [CostInsightsApi](https://github.com/backstage/backstage/blob/master/plugins/cost-insights/src/api/CostInsightsApi.ts) interface. Create your own or [use a template](https://github.com/backstage/backstage/blob/master/plugins/cost-insights/src/example/templates/CostInsightsClient.ts) to get started.

 ```ts
 // path/to/CostInsightsClient.ts
@@ -36,7 +34,9 @@ import { CostInsightsApi } from '@backstage/plugin-cost-insights';
 export class CostInsightsClient implements CostInsightsApi { ... }
 ```

-3. Import the client and the CostInsights plugin API to your Backstage instance.
+**Note:** We've briefly explored using the AWS Cost Explorer API to implement a Cost Insights client. Learn more about our findings [here](https://github.com/backstage/backstage/blob/master/plugins/cost-insights/contrib/aws-cost-explorer-api.md).
+
+3. Import the client and the Cost Insights plugin API to your Backstage instance.

 ```ts
 // packages/app/src/api.ts
@@ -43,4 +43,22 @@ describe('<ActionItemCard/>', () => {
    expect(rendered.getByText(alert.title)).toBeInTheDocument();
    expect(rendered.getByText(alert.subtitle)).toBeInTheDocument();
  });
+
+  it('renders custom title elements', async () => {
+    const rendered = await renderInTestApp(
+      <MockScrollProvider>
+        <ActionItemCard
+          alert={{
+            ...alert,
+            title: <span>Foo</span>,
+            subtitle: <span>Bar</span>,
+          }}
+          avatar={<div>1</div>}
+        />
+      </MockScrollProvider>,
+    );
+
+    expect(rendered.getByText('Foo')).toBeInTheDocument();
+    expect(rendered.getByText('Bar')).toBeInTheDocument();
+  });
 });
@@ -41,14 +41,33 @@ describe('<AlertInsightsSection/>', () => {
        onAccept={jest.fn()}
      />,
    );
-    expect(getByText(mockAlert.title)).toBeInTheDocument();
-    expect(getByText(mockAlert.subtitle)).toBeInTheDocument();
+    expect(getByText(mockAlert.title as string)).toBeInTheDocument();
+    expect(getByText(mockAlert.subtitle as string)).toBeInTheDocument();
    expect(getByText('View Instructions')).toBeInTheDocument();
    expect(queryByText('Snooze')).not.toBeInTheDocument();
    expect(queryByText('Accept')).not.toBeInTheDocument();
    expect(queryByText('Dismiss')).not.toBeInTheDocument();
  });

+  it('renders custom title elements', () => {
+    const { getByText } = renderInContext(
+      <AlertInsightsSection
+        alert={{
+          ...mockAlert,
+          title: <span>Foo</span>,
+          subtitle: <span>Bar</span>,
+        }}
+        number={1}
+        onSnooze={jest.fn()}
+        onDismiss={jest.fn()}
+        onAccept={jest.fn()}
+      />,
+    );
+
+    expect(getByText('Foo')).toBeInTheDocument();
+    expect(getByText('Bar')).toBeInTheDocument();
+  });
+
  it('Hides instructions button if url is not provided', () => {
    const alert: Alert = {
      ...mockAlert,
@@ -50,8 +50,8 @@ describe('<AlertStatusSummary />', () => {
      </MockScrollProvider>,
    );
    [mockSnoozed, mockAccepted, mockDismissed].forEach(a => {
-      expect(getByText(a.title)).toBeInTheDocument();
-      expect(getByText(a.subtitle)).toBeInTheDocument();
+      expect(getByText(a.title as string)).toBeInTheDocument();
+      expect(getByText(a.subtitle as string)).toBeInTheDocument();
      expect(getByRole('img', { name: a.status })).toBeInTheDocument();
    });
  });
@@ -17,6 +17,7 @@
 import React from 'react';
 import pluralize from 'pluralize';
 import { KubernetesMigrationAlertCard } from '../components';
+import { Lifecycle } from '@backstage/core';
 import { CostInsightsApi } from '../../api';
 import {
  Alert,
@@ -87,11 +88,13 @@ export class KubernetesMigrationAlert implements KubernetesMigrationApi {
  }

  get title() {
-    return `Consider migrating ${pluralize(
-      'service',
-      this.data.services.length,
-      true,
-    )} to Kubernetes.`;
+    return (
+      <span>
+        Consider migrating{' '}
+        {pluralize('service', this.data.services.length, true)} to Kubernetes{' '}
+        <Lifecycle shorthand />
+      </span>
+    );
  }

  get element() {
@@ -0,0 +1,101 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+  This is a copy-pastable client template to get up and running quickly.
+  API Reference:
+  https://github.com/backstage/backstage/blob/master/plugins/cost-insights/src/api/CostInsightsApi.ts
+*/
+
+// IMPORTANT: Remove the lines below to enable type checking and linting
+// @ts-nocheck
+/* eslint-disable import/no-extraneous-dependencies */
+
+import {
+  CostInsightsApi,
+  ProductInsightsOptions,
+  Alert,
+  Cost,
+  Entity,
+  Group,
+  MetricData,
+  Project,
+} from '@backstage/plugin-cost-insights';
+
+export class CostInsightsClient implements CostInsightsApi {
+
+  async getLastCompleteBillingDate(): Promise<string> {
+    return '2021-01-01'; // YYYY-MM-DD
+  }
+
+  async getUserGroups(userId: string): Promise<Group[]> {
+    return [];
+  }
+
+  async getGroupProjects(group: string): Promise<Project[]> {
+    return [];
+  }
+
+  async getAlerts(group: string): Promise<Alert[]> {
+    return [];
+  }
+
+  async getDailyMetricData(metric: string, intervals: string): Promise<MetricData> {
+    return {
+      id: 'remove-me',
+      format: 'number',
+      aggregation: [],
+      change: {
+        ratio: 0,
+        amount: 0
+      }
+    }
+  }
+
+  async getGroupDailyCost(group: string, intervals: string): Promise<Cost> {
+    return {
+      id: 'remove-me',
+      aggregation: [],
+      change: {
+        ratio: 0,
+        amount: 0
+      }
+    }
+  }
+
+  async getProjectDailyCost(project: string, intervals: string): Promise<Cost> {
+    return {
+      id: 'remove-me',
+      aggregation: [],
+      change: {
+        ratio: 0,
+        amount: 0
+      }
+    }
+  }
+
+  async getProductInsights(options: ProductInsightsOptions): Promise<Entity> {
+    return {
+      id: 'remove-me',
+      aggregation: [0, 0],
+      change: {
+        ratio: 0,
+        amount: 0
+      },
+      entities: {}
+    }
+  }
+}
@@ -34,8 +34,8 @@ import { Maybe } from './Maybe';
 */

 export type Alert = {
-  title: string;
-  subtitle: string;
+  title: string | JSX.Element;
+  subtitle: string | JSX.Element;
  element?: JSX.Element;
  status?: AlertStatus;
  url?: string;
@@ -76,7 +76,7 @@ export function InDepth() {
            </Typography>

            <Typography variant="body2" color="textSecondary">
-              <strong>Longest release</strong>:{' '}
+              <strong>Lengthiest release</strong>:{' '}
              <LongestReleaseTime averageReleaseTime={averageReleaseTime} />
            </Typography>
          </Box>
@@ -19,7 +19,10 @@ import {
  ScmIntegrationRegistry,
 } from '@backstage/integration';
 import { Octokit } from '@octokit/rest';
-import { initRepoAndPush } from '../../../stages/publish/helpers';
+import {
+  enableBranchProtectionOnDefaultRepoBranch,
+  initRepoAndPush,
+} from '../../../stages/publish/helpers';
 import { getRepoSourceDirectory, parseRepoUrl } from './util';
 import { createTemplateAction } from '../../createTemplateAction';

@@ -171,7 +174,7 @@ export function createPublishGithubAction(options: {
              description: description,
            });

-      const { data } = await repoCreationPromise;
+      const { data: newRepo } = await repoCreationPromise;
      if (access?.startsWith(`${owner}/`)) {
        const [, team] = access.split('/');
        await client.teams.addOrUpdateRepoPermissionsInOrg({
@@ -212,8 +215,8 @@ export function createPublishGithubAction(options: {
        }
      }

-      const remoteUrl = data.clone_url;
-      const repoContentsUrl = `${data.html_url}/blob/master`;
+      const remoteUrl = newRepo.clone_url;
+      const repoContentsUrl = `${newRepo.html_url}/blob/master`;

      await initRepoAndPush({
        dir: getRepoSourceDirectory(ctx.workspacePath, ctx.input.sourcePath),
@@ -225,6 +228,18 @@ export function createPublishGithubAction(options: {
        logger: ctx.logger,
      });

+      try {
+        await enableBranchProtectionOnDefaultRepoBranch({
+          owner,
+          client,
+          repoName: newRepo.name,
+        });
+      } catch (e) {
+        throw new Error(
+          `Failed to add branch protection to '${newRepo.name}', ${e}`,
+        );
+      }
+
      ctx.output('remoteUrl', remoteUrl);
      ctx.output('repoContentsUrl', repoContentsUrl);
    },
@@ -60,6 +60,7 @@ describe('JobProcessor', () => {
          },
        },
      },
+      owner: 'example@email.com',
    },
  };

@@ -55,6 +55,7 @@ describe('Helpers', () => {
              },
            },
          },
+          owner: 'team-d@example.com',
        },
      };

@@ -103,6 +104,7 @@ describe('Helpers', () => {
              },
            },
          },
+          owner: 'team-b@example.com',
        },
      };

@@ -151,6 +153,7 @@ describe('Helpers', () => {
              },
            },
          },
+          owner: 'team-a@example.com',
        },
      };

@@ -198,6 +201,7 @@ describe('Helpers', () => {
              },
            },
          },
+          owner: 'team-b@example.com',
        },
      };

@@ -243,6 +247,7 @@ describe('Helpers', () => {
              },
            },
          },
+          owner: 'team-c@example.com',
        },
      };

@@ -15,7 +15,10 @@
 */

 import { PublisherBase, PublisherOptions, PublisherResult } from './types';
-import { initRepoAndPush } from './helpers';
+import {
+  enableBranchProtectionOnDefaultRepoBranch,
+  initRepoAndPush,
+} from './helpers';
 import {
  GitHubIntegrationConfig,
  GithubCredentialsProvider,
@@ -26,6 +29,7 @@ import path from 'path';

 export type RepoVisibilityOptions = 'private' | 'internal' | 'public';

+/** @deprecated use createPublishGithubAction instead */
 export class GithubPublisher implements PublisherBase {
  static async fromConfig(
    config: GitHubIntegrationConfig,
@@ -99,6 +103,17 @@ export class GithubPublisher implements PublisherBase {
      /\.git$/,
      '/blob/master/catalog-info.yaml',
    );
+
+    try {
+      await enableBranchProtectionOnDefaultRepoBranch({
+        owner,
+        client,
+        repoName: name,
+      });
+    } catch (e) {
+      throw new Error(`Failed to add branch protection to '${name}', ${e}`);
+    }
+
    return { remoteUrl, catalogInfoUrl };
  }

@@ -130,7 +145,7 @@ export class GithubPublisher implements PublisherBase {
            description,
          });

-    const { data } = await repoCreationPromise;
+    const { data: newRepo } = await repoCreationPromise;

    try {
      if (access?.startsWith(`${owner}/`)) {
@@ -156,6 +171,7 @@ export class GithubPublisher implements PublisherBase {
        `Failed to add access to '${access}'. Status ${e.status} ${e.message}`,
      );
    }
-    return data?.clone_url;
+
+    return newRepo.clone_url;
  }
 }
@@ -17,6 +17,7 @@
 import globby from 'globby';
 import { Logger } from 'winston';
 import { Git } from '@backstage/backend-common';
+import { Octokit } from '@octokit/rest';

 export async function initRepoAndPush({
  dir,
@@ -67,3 +68,50 @@ export async function initRepoAndPush({
    remote: 'origin',
  });
 }
+
+type BranchProtectionOptions = {
+  client: Octokit;
+  owner: string;
+  repoName: string;
+  isRetry?: boolean;
+};
+
+export const enableBranchProtectionOnDefaultRepoBranch = async ({
+  repoName,
+  client,
+  owner,
+}: BranchProtectionOptions): Promise<void> => {
+  const tryOnce = () => {
+    return client.repos.updateBranchProtection({
+      mediaType: {
+        /**
+         * 👇 we need this preview because allowing a custom
+         * reviewer count on branch protection is a preview
+         * feature
+         *
+         * More here: https://docs.github.com/en/rest/overview/api-previews#require-multiple-approving-reviews
+         */
+        previews: ['luke-cage-preview'],
+      },
+      owner,
+      repo: repoName,
+      branch: 'master',
+      required_status_checks: { strict: true, contexts: [] },
+      restrictions: null,
+      enforce_admins: true,
+      required_pull_request_reviews: { required_approving_review_count: 1 },
+    });
+  };
+
+  try {
+    await tryOnce();
+  } catch (e) {
+    if (!e.message.includes('Branch not found')) {
+      throw e;
+    }
+
+    // GitHub has eventual consistency. Fail silently, wait, and try again.
+    await new Promise(resolve => setTimeout(resolve, 600));
+    await tryOnce();
+  }
+};
@@ -32,24 +32,42 @@ import React from 'react';
 import WarningIcon from '@material-ui/icons/Warning';
 import { generatePath } from 'react-router';
 import { rootRouteRef } from '../../routes';
-import { TemplateEntityV1alpha1 } from '@backstage/catalog-model';
+import {
+  TemplateEntityV1alpha1,
+  Entity,
+  RELATION_OWNED_BY,
+} from '@backstage/catalog-model';
 import { FavouriteTemplate } from '../FavouriteTemplate/FavouriteTemplate';
+import {
+  getEntityRelations,
+  EntityRefLinks,
+} from '@backstage/plugin-catalog-react';

-const useStyles = makeStyles({
+const useStyles = makeStyles(theme => ({
  cardHeader: {
    position: 'relative',
  },
  title: {
    backgroundImage: ({ backgroundImage }: any) => backgroundImage,
  },
-  description: {
+  box: {
    overflow: 'hidden',
    textOverflow: 'ellipsis',
    display: '-webkit-box',
    '-webkit-line-clamp': 10,
    '-webkit-box-orient': 'vertical',
+    paddingBottom: '0.8em',
  },
-});
+  label: {
+    color: theme.palette.text.secondary,
+    textTransform: 'uppercase',
+    fontSize: '0.65rem',
+    fontWeight: 'bold',
+    letterSpacing: 0.5,
+    lineHeight: 1,
+    paddingBottom: '0.2rem',
+  },
+}));

 const useDeprecationStyles = makeStyles(theme => ({
  deprecationIcon: {
@@ -116,7 +134,10 @@ export const TemplateCard = ({ template, deprecated }: TemplateCardProps) => {
  const backstageTheme = useTheme<BackstageTheme>();
  const rootLink = useRouteRef(rootRouteRef);
  const templateProps = getTemplateCardProps(template);
-
+  const ownedByRelations = getEntityRelations(
+    template as Entity,
+    RELATION_OWNED_BY,
+  );
  const themeId = pageTheme[templateProps.type] ? templateProps.type : 'other';
  const theme = backstageTheme.getPageTheme({ themeId });
  const classes = useStyles({ backgroundImage: theme.backgroundImage });
@@ -135,13 +156,27 @@ export const TemplateCard = ({ template, deprecated }: TemplateCardProps) => {
          classes={{ root: classes.title }}
        />
      </CardMedia>
-      <CardContent>
+      <CardContent style={{ display: 'grid' }}>
+        <Box className={classes.box}>
+          <Typography variant="body2" className={classes.label}>
+            Description
+          </Typography>
+          {templateProps.description}
+        </Box>
+        <Box className={classes.box}>
+          <Typography variant="body2" className={classes.label}>
+            Owner
+          </Typography>
+          <EntityRefLinks entityRefs={ownedByRelations} defaultKind="Group" />
+        </Box>
        <Box>
+          <Typography variant="body2" className={classes.label}>
+            Tags
+          </Typography>
          {templateProps.tags?.map(tag => (
            <Chip size="small" label={tag} key={tag} />
          ))}
        </Box>
-        <Box className={classes.description}>{templateProps.description}</Box>
      </CardContent>
      <CardActions>
        <Button