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

[LintDiff] Deploy pending azure-openapi-validator changes from staging to prod #6071

Closed
konrad-jamrozik opened this issue May 1, 2023 · 4 comments
Assignees
Labels
Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo.

Comments

@konrad-jamrozik
Copy link
Contributor

konrad-jamrozik commented May 1, 2023

As required by this support request:

The LintDiff PR CI check running on https://github.com/Azure/azure-rest-api-specs hasn't got its changes deployed from staging to prod since months.

Investigation shows doing it in a safe, correct way is nontrivial. This task is about accomplishing this.

Related task o document this process for further deployments:

Note there is a private Teams chat between Brett DeFoy, myself and few other folks to help with figuring out some specific pending changes to get deployed on request by Brett, per:

@konrad-jamrozik konrad-jamrozik added Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo. labels May 1, 2023
@konrad-jamrozik konrad-jamrozik self-assigned this May 1, 2023
@konrad-jamrozik
Copy link
Contributor Author

konrad-jamrozik commented May 1, 2023

A plan how to deploy staging LintDiff changes to prod

The goal

We want to deploy the LintDiff ruleset currently running as part of the ~[Staging] Swagger LintDiff PR CI check to Swagger LintDiff PR CI check. We want to do it in a safe, reversible manner. That is, if the changes we deploy will break prod check, we want to be able to revert it quickly.

What needs to be done

The ruleset package.json needs to be updated from 1.0.1 to 1.1.0 and released to the npm @microsoft.azure/openapi-validator-rulesets package via the azure-openapi-validator to npm release pipeline.

However, we cannot do this outright, as doing so would immediately update the ruleset used in prod check and leave no easy way of reverting this change. This is because the prod check uses v2.0.0 of @microsoft.azure/openapi-validator (recent example) which has following dependency: "@microsoft.azure/openapi-validator-rulesets": "^1.0.0", which evaluates to 1.x. In addition, there are changes in azure-openapi-validator main pending deployment, which would make any reverts even harder. The version 2.0.0 was published on 11/7/2022, 6 months ago, and there have been changes since then as can be seen from the commit history. There exists similar problem with the @microsoft.azure/openapi-validator-core package on which @microsoft.azure/openapi-validator depends. The latest -core version was published on 11/7/2022, 6 months ago, but there were commits since then.

Lack of easy reversal means we need to fix that first. Thus, we first need to update the azure-openapi-validator package.json to depend exactly on ruleset v1.0.1 and the update the openapi-validator-core package.json to 1.0.1 and deploy these changes, thus also deploying the changes pending in main to the openapi-validator itself and openapi-validator-core.

Next we shall publish ruleset v1.1.0, then deploy azure-openapi-validator that depends on ruleset v1.1.0. This way we can easily revert if anything goes wrong, by changing and deploying LINT_VERSION of the azure-openapi-validator package used.

The ruleset v1.0.1 was published 11/24/2022, 5 months ago, and we can see from the commit history there were many commits made since then. These changes already run as part of ~[Staging] Swagger LintDiff check, but nevertheless deploying them to prod introduces risk due to increased severity of a failure.

The above considerations can be summarized in following table breaking down the plan by stages:

plan stage azure-openapi-validator azure-openapi-core in effect ruleset dependency latest ruleset ruleset in effect
0 - current 2.0.0 1.0.0 1.x 1.0.1 1.0.1
1 2.0.1 1.0.1 1.0.1 1.0.1 1.0.1
2 2.0.1 1.0.1 1.0.1 1.1.0 1.0.1
3 2.1.0 1.0.1 1.1.0 1.1.0 1.1.0

The plan step by step

Stage 1:

1.1. Submit a PR to azure-openapi-validator affixing the ruleset dependency to be 1.0.1 instead of ~1.0.1 and updating -core to 1.0.1. Note the ruleset dependency is different from the deployed dependency which has ^1.0.0, but the difference doesn't matter as we change it to 1.0.1.
1.2. Merge the PR and deploy 2.0.1 of @microsoft.azure/openapi-validator to npm.
1.3. Update LINT_VERSION to 2.0.1 and deploy the change. Verify the Swagger LintDiff prod CI check still works as expected.

Stage 2:

2.1. Submit a PR to azure-openapi-validator bumping the ruleset version to 1.1.0.
2.2. Merge the PR and deploy 1.1.0 of @microsoft.azure/openapi-validator-rulesets to npm.

Note there is no easy way to verify that the Swagger LintDiff prod CI check still uses 1.0.1 ruleset, not 1.1.0, so we will likely skip doing such verification.

Stage 3:

3.1. Submit a PR to azure-openapi-validator bumping its version to 2.1.0 and affixing the ruleset dependency to be 1.1.0 instead of 1.0.1.
3.2. Merge the PR and deploy 2.1.0 of @microsoft.azure/openapi-validator to npm.
3.3 OBSOLETE: See Plan amendment below. Update LINT_VERSION to 2.1.0 and deploy the change. Verify the Swagger LintDiff prod CI check now uses the new ruleset. We can do it by observing at least one occurrence of one of the new rules. Some example rules we expect to now see are given in PR 424, PR 436 and PR 449.

Plan amendment, added 5/9/2023:

Due to issues with rules-to-be-deployed as explained here:

We couldn't deploy the pending ruleset changes as-is. We had to disable some rules that return false positive. We also made few improvements along the way. All of these changes have been captured by these PRs:

As a result, we have additional stage, stage 4, of the plan:

Stage 4:

4.1. Submit & merge a set of PRs to azure-openapi-validator as listed above. This includes bumping its version 2.1.1, affixing the ruleset dependency to be 1.1.0 instead of 1.1.1, and bumping the ruleset version to 1.1.1.
4.2. Ensure these changes have been released as beta npm packages.
4.3. Deploy the updated @microsoft.azure/openapi-validator and @microsoft.azure/openapi-validator-rulesetsto npm.
4.4. Update LINT_VERSION to 2.1.1 and deploy the change.

Stage 5:

  1. Verify the Swagger LintDiff prod CI check now uses the new ruleset.

@weshaggard
Copy link
Member

@konrad-jamrozik sounds like a reasonable plan.

@konrad-jamrozik
Copy link
Contributor Author

konrad-jamrozik commented May 1, 2023

Plan execution status

Stage 1:

1.1:

1.2.

1.3:

2.1:

2.2:

3.1:

3.2:

3.3: SKIPPED (see plan amendment)

4.1:

4.3:

4.4:

5:

@konrad-jamrozik
Copy link
Contributor Author

konrad-jamrozik commented May 5, 2023

We need to delay deploying the rules in staging to prod due to users reporting multiple false positives:

Here are the rules identified to produce false positives. They need to be disabled before we deploy anything to production:

  • ResourceMustReferenceCommonTypes
  • LROPostFinalStateViaProperty (the plan is to remove any rules that address 'final-state-via ')
  • ProvisioningStateMustBeReadOnly

Main issue to track:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo.
Projects
Archived in project
Status: 🎊 Closed
Development

No branches or pull requests

2 participants