Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

TT 3695 - Fix Security Policy Migration #540

Merged
merged 23 commits into from
Dec 22, 2022
Merged

Conversation

buraksekili
Copy link
Collaborator

@buraksekili buraksekili commented Nov 30, 2022

Description

The migration steps described in this documentation do not work as expected. Setting the id field in the CR to the _id field of the existing Policy has some issues.

Related Issue

Resolves #204
https://tyktech.atlassian.net/browse/TT-3695

Motivation and Context

Once you migrate the existing policy from Dashboard to k8s, k8s state should include details of the spec that exists on Dashboard, and from now on, the source of truth must become k8s. Operator must restore subsequent changes from Dashboard in the next reconciliations.

Test Coverage For This Change

Added test cases to cover:

  • Migration of an existing Security Policy from Dashboard to k8s
  • Ensuring k8s is the source of truth.
    • Drifts (updates/deletes) from Dashboard should be restored based on the k8s state in the next reconciliation.
  • Creating a simple SecurityPolicy should update ApiDefinition's status if that SecurityPolicy refers to it
    • Ensure that corresponding resources are created on Tyk
  • Deleting this SecurityPolicy must update ApiDefinition status.
    • Ensure that the corresponding SecurityPolicy is deleted from Tyk as well

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If PRing from your fork, don't come from your master!
  • Make sure you are making a pull request against our master branch (left side). Also, it would be best if you started your change off our latest master.
  • Make sure you are updating CHANGELOG.md based on your changes.
  • My change requires a change to the documentation.
    • If you've changed APIs, describe what needs to be updated in the documentation.
  • I have updated the documentation accordingly.
  • If you've changed API models, please update CRDs.
    • make manifests
    • make helm
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Check your code additions will not fail linting checks:
    • gofmt -s -w .
    • go vet ./...
    • golangci-lint run

Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
it will return SecurityPolicy spec.

Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
…gies/tyk-operator into TT-3695/fix-policy-migration
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
@buraksekili buraksekili marked this pull request as ready for review December 5, 2022 21:53
@buraksekili buraksekili requested a review from a team as a code owner December 5, 2022 21:53
@buraksekili buraksekili requested review from komalsukhani and removed request for a team December 5, 2022 21:53
@caroltyk caroltyk added this to the v0.13.0 milestone Dec 6, 2022
…urityPolicy. Add new e2e tests cases for migration

Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Controller.

Signed-off-by: Burak Sekili <buraksekili@gmail.com>
@komalsukhani
Copy link
Collaborator

@buraksekili Thank you for resolving this complicated bug! It looks good to me.

Signed-off-by: Burak Sekili <buraksekili@gmail.com>
…gies/tyk-operator into TT-3695/fix-policy-migration
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
…ison with already-defined function. Compare previous and new ID after restoring from drifts. Remove redundant conditions in wait.For() function

Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Signed-off-by: Burak Sekili <buraksekili@gmail.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@singhpr singhpr merged commit 03e610c into master Dec 22, 2022
@singhpr singhpr deleted the TT-3695/fix-policy-migration branch December 22, 2022 08:15
buger pushed a commit that referenced this pull request May 22, 2024
* Add if check to understand the existence of mongo id

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Update CHANGELOG

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Update current CR based on existing policy

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Update create method of SecurityPolicy Reconciler. So that, from now on
it will return SecurityPolicy spec.

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* use CR's AccessRightsArray field instead of generating it

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* add initial e2e tests for securitypolicy controller

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* fix linter issue, add additional error checks for type assertions

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Add a logic to recover from unintended deletes from Dashboard for SecurityPolicy. Add new e2e tests cases for migration

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Fix issues on v3.2

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* generate random name for securitypolicy

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Wait for resource is updated properly

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Add new test cases to cover different scenarios of SecurityPolicy
Controller.

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Revert changes and update tests accordingly

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Fix linter issue - remove extra new line

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Use unused StatusID in comparison function. Replace duplicated comparison with already-defined function. Compare previous and new ID after restoring from drifts. Remove redundant conditions in wait.For() function

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* Update tests cases to prevent direct comparison between IDs

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

* return err if Tyk fails to reload

Signed-off-by: Burak Sekili <buraksekili@gmail.com>

Signed-off-by: Burak Sekili <buraksekili@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TT-3695] Validate/Document managing of policies created on the dashboard
4 participants