Allow repositories to override CLA bot configuration

[maoo]

Currently, the CLA bot tries to resolve configuration at org level (looking for a clabot-config repository), otherwise it falls back to repo configuration.

Code is on https://github.com/finos/cla-bot/blob/develop/src/index.js#L155-L182

  let orgConfig;
  try {
    logger.info("Attempting to obtain organisation level .clabot file URL");
    orgConfig = await getOrgConfig(webhook);
    logger.info("Organisation configuration found!");
  } catch (e) {
    logger.info(
      "Organisation configuration not found, resolving .clabot URL at project level"
    );
    orgConfig = await getReadmeUrl(webhook);
  }

  logger.info(
    `Obtaining .clabot configuration file from ${
      orgConfig.download_url.split("?")[0]
    }`
  );

  const config = await getFile(orgConfig);

  if (!is.json(config)) {
    logger.error("The .clabot file is not valid JSON");
    await setStatus(webhook, headSha, "error", logFile);
    throw new Error("The .clabot file is not valid JSON");
  }

  // merge with default config options
  const botConfig = Object.assign({}, defaultConfig, config);

Side note, getReadmeUrl should be probably be renamed to getRepoConfig.

I'd like to propose the introduction of 2 parameters as part of the .clabot definition:

• allowRepoConfigOverride (true/false, defaults to false, to preserve backward compatibility), tries to resolve configuration at repository level
• allowedReposConfigOverrides (string, comma-separated values), lists the name of GitHub repositories that are allowed to use their own configuration; this configuration is ignored if allowRepoConfigOverride is false.

Draft code below (not tested yet):

const loadConfigFile = (configFile, webhook, headSha, logFile, defaultConfig) => {
  logger.info(
    `Obtaining .clabot configuration file from ${
      configFile.download_url.split("?")[0]
    }`
  );

  const config = await getFile(configFile);

  if (!is.json(config)) {
    logger.error("The .clabot file is not valid JSON");
    await setStatus(webhook, headSha, "error", logFile);
    throw new Error("The .clabot file is not valid JSON");
  }

  // merge with default config options
  return Object.assign({}, defaultConfig, config);
}

....

  const repoName = pullRequestUrl.split("/")[5];

....

  let repoConfig;
  let isOrgConfig = false;
  try {
    logger.info("Attempting to obtain organisation level .clabot file URL");
    repoConfig = await getOrgConfig(webhook);
    isOrgConfig = true;
    logger.info("Organisation configuration found!");
  } catch (e) {
    logger.info(
      "Organisation configuration not found, resolving .clabot URL at project level"
    );
    repoConfig = await getRepoConfig(webhook);
  }

  // merge with default config options
  let botConfig = loadConfigFile(repoConfig, webhook, headSha, logFile, defaultConfig);

  if (isOrgConfig && botConfig.allowRepoConfigOverride) {
    if (
      botConfig.allowedReposConfigOverrides &&
      !botConfig.allowedReposConfigOverrides.split(',').includes(repoName)) {
        logger.warn(`Repo ${repoName} is not allowed to override configurations; using org-level configuration instead`);
    } else {
      repoConfig = await getRepoConfig(webhook);
      botConfig = loadConfigFile(repoConfig, webhook, headSha, logFile, defaultConfig);
    }
  }

@ColinEberhardt - happy to send over a PR, if you like the idea!

[mcleo-d]

@maoo - Thanks for opening this issue and coming up with such a great idea!

It would be great to bring this suggestion to the FINOS Open Developer Platform working group for discussion and prioritisation before moving forward with the change.

I have created the backlog item here ... https://finosfoundation.atlassian.net/browse/ODP-110

In particular, we should bring answers to the following ...

• The benefits the change brings to OSS project teams.
• The testing strategy prior to merge and deploy.
• The potential risks of making the change to current integrating teams.
• How the risks above are being mitigated.

Thanks once again and I look forward to discussing soon.

James.

[ColinEberhardt]

Hi @maoo so what you are basically after is the ability for repos to have their own config that overrides an organisation-level config?

In that case, there's a very simple change that can be made without needing extra configuration parameters. We simply give repository-level config files a higher level of precedence than organisation-level ones.

In other words

1. fetch org-level config
2. fetch repo-level config

If (2) exists, that takes precedence

WDYT?

[maoo]

Thanks for the feedback.

@mcleo-d +1 , thanks for adding the item in the ODP backlog

In other words

1. fetch org-level config
2. fetch repo-level config If (2) exists, that takes precedence

@ColinEberhardt - that is basically the idea, but adding the ability at org-level to 1) decide whether the repo override feature is enabled or not and 2) decide which repos are allowed.

If we don't make this configurable, this change would cause a different behaviour of the existing instances (assuming a repo defines a .clabot file, maybe unknowingly).

[maoo]

Closing this issue, as FINOS doesn't need this feature any longer; feel free to reopen if there's anyone else interested.