Skip to content
Snippets Groups Projects

Fix IdentityProviderPolicy unlink rule

Merged Jarka Košanová requested to merge 291007-identity-provider-fix into master

What does this MR do and why?

There was a problem with a rule in IdentityProviderPolicy. The usage of && is not allowed in a rule in declarative policy and can lead to wrong results.

The specs were not written correctly and therefore not failing.

Rather than fixing the policy, we can remove it because this policy class is used for abilities related to group-managed accounts, which has been EOL-d. See &8518

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #291007 (closed)

Edited by Jessie Young

Merge request reports

Merged results pipeline #1010549000 passed

Pipeline: GitLab

#1010552833

    Pipeline: E2E GDK

    #1010559473

      Merged results pipeline passed for 84c357c1

      Test coverage 82.47% (15.72%) from 2 jobs
      Approval is optional

      Merged by Marc ShawMarc Shaw 1 year ago (Sep 20, 2023 7:52am UTC)

      Merge details

      • Changes merged into master with 089dbd3c (commits were squashed).
      • Deleted the source branch.
      • Auto-merge enabled

      Pipeline #1010620795 passed

      Pipeline passed for 089dbd3c on master

      Test coverage 66.81% (15.72%) from 2 jobs
      10 environments impacted.

      Activity

      Filter activity
      • Approvals
      • Assignees & reviewers
      • Comments (from bots)
      • Comments (from users)
      • Commits & branches
      • Edits
      • Labels
      • Lock status
      • Mentions
      • Merge request status
      • Tracking
    • :warning: @jarka Some end-to-end (E2E) tests have been selected based on the stage label on this MR.

      Please start the trigger-omnibus-and-follow-up-e2e job in the qa stage and ensure the tests in follow-up-e2e:package-and-test-ee pipeline are passing before this MR is merged. (The E2E test pipeline is computationally intensive and we cannot afford running it automatically for all pushes/rebases. Therefore, this job must be triggered manually after significant changes at least once.)

      If you would like to run all E2E tests, please apply the pipeline:run-all-e2e label and trigger a new pipeline. This will run all tests in e2e:package-and-test pipeline.

      The E2E test jobs are allowed to fail due to flakiness. For the list of known failures please refer to the latest pipeline triage issue.

      Once done, please apply the :white_check_mark: emoji on this comment.

      For any questions or help in reviewing the E2E test results, please reach out on the internal #quality Slack channel.

    • Allure report

      allure-report-publisher generated test report!

      e2e-test-on-gdk: :exclamation: test report for a4bf84cc

      expand test summary
      +------------------------------------------------------------------+
      |                          suites summary                          |
      +-------------+--------+--------+---------+-------+-------+--------+
      |             | passed | failed | skipped | flaky | total | result |
      +-------------+--------+--------+---------+-------+-------+--------+
      | Plan        | 51     | 0      | 0       | 0     | 51    | ✅     |
      | Data Stores | 20     | 0      | 0       | 1     | 20    | ❗     |
      | Govern      | 36     | 0      | 0       | 0     | 36    | ✅     |
      | Create      | 38     | 0      | 0       | 0     | 38    | ✅     |
      | Manage      | 12     | 0      | 1       | 0     | 13    | ✅     |
      | Verify      | 8      | 0      | 0       | 0     | 8     | ✅     |
      +-------------+--------+--------+---------+-------+-------+--------+
      | Total       | 165    | 0      | 1       | 1     | 166   | ❗     |
      +-------------+--------+--------+---------+-------+-------+--------+
      Edited by Ghost User
    • :wave: @jarka, please can you answer the question: Should this have a feature flag? to help with code review for the Authentication and Authorization group.

      This nudge was added by this triage-ops policy.

    • @jessieay could you please review?

    • Jarka Košanová requested review from @jessieay

      requested review from @jessieay

    • added 1 commit

      • 98efd935 - Remove policies related to group-managed accounts

      Compare with previous version

    • Jessie Young changed the description

      changed the description

    • Jessie Young changed the description

      changed the description

    • Jessie Young approved this merge request

      approved this merge request

    • Jessie Young requested review from @marc_shaw

      requested review from @marc_shaw

    • :wave: @jessieay, thanks for approving this merge request.

      This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.

      For more info, please refer to the following links:

    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Please register or sign in to reply
      Loading