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

Add an ad-hoc Tyk API call to verify non-existent Policies in delete method#558

Merged
komalsukhani merged 2 commits intomasterfrom
fix/check-policy-existence
Jan 11, 2023
Merged

Add an ad-hoc Tyk API call to verify non-existent Policies in delete method#558
komalsukhani merged 2 commits intomasterfrom
fix/check-policy-existence

Conversation

@buraksekili
Copy link

Description

The CI fails due to failures that occurred in security policy deletion. As of today, the Policy API on GW does not return 404 while deleting non-existent Policies. Therefore, Operator tries to reconcile and cannot understand the existence of Policies. Until a fix is released on GW, this PR adds an additional step to verify the existence of Policy objects on Tyk level.

Related Issue

https://github.com/TykTechnologies/tyk-operator/actions/runs/3873378749/jobs/6603330308#step:8:29

Motivation and Context

Test Coverage For This Change

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

…n operation

Signed-off-by: Burak Sekili <buraksekili@gmail.com>
@buraksekili buraksekili marked this pull request as ready for review January 10, 2023 07:12
@buraksekili buraksekili requested a review from a team as a code owner January 10, 2023 07:12
@buraksekili buraksekili requested review from komalsukhani and removed request for a team January 10, 2023 07:12
// If the Policy does not exist, no need to reconcile again. Otherwise, return err and reconcile.
_, errTyk := klient.Universal.Portal().Policy().Get(ctx, policy.Status.PolID)
if opclient.IsNotFound(errTyk) {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning here, we should continue to perform operations defined below if.

err := r.updateStatusOfLinkedAPIs(ctx, policy, true)
	if err != nil {
		return err
	}

	err = klient.Universal.HotReload(ctx)
	if err != nil {
		r.Log.Error(err, "Failed to hot-reload Tyk after deleting a Policy",
			"Policy", client.ObjectKeyFromObject(policy),
		)

		return err
	}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated can you please re-review again? Now, the execution continues only if errTyk is 404. The other response codes 200 or even 500 make operator continue working.

…oes not found

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 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@komalsukhani komalsukhani merged commit f320d3c into master Jan 11, 2023
@komalsukhani komalsukhani deleted the fix/check-policy-existence branch January 11, 2023 12:19
buger pushed a commit that referenced this pull request May 22, 2024
…method (#558)

* Add an ad-hoc Tyk API call to verify non-existent Policies in deletion operation

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

* Update if condition flow; so that the execution continues if Policy does not found

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.

2 participants