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

Support for the link to the rule set deploy status #179

Conversation

philippschulte
Copy link
Member

@philippschulte philippschulte commented May 11, 2020

This PR is necessary in order to be able to use the link of the rule set deploy status in waflyctl.

The request methods for GET/service/service_id/wafs/waf_id/ruleset and PATCH/service/service_id/wafs/waf_id/ruleset share the same response struct. However, the responses of these endpoints are actually differnet. The PATCH endpoint doesn't support the vcl and the last_push fields.

I am just adding the Link field to the shared Ruleset response struct. This field will be empty and useless for the GET/service/service_id/wafs/waf_id/ruleset request function but I don't think this matters because the response struct for PATCH/service/service_id/wafs/waf_id/ruleset also includes empty and useless fields as well. The jsonapi package doesn't allow me to decode the Link field and therefore I am using the standard library for the job.

I understand that this is not the nicest implementation but it doesn't introduce any breaking changes.

@philippschulte philippschulte force-pushed the pschulte/support-for-the-link-to-the-rule-set-deploy-status branch from 423dc5a to 1ccfb1d Compare May 11, 2020 21:13
Copy link
Member

@phamann phamann left a comment

Choose a reason for hiding this comment

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

This is not the first time i've had to battle with the google/jsonapi package, and I'm sure it won't be the last.

Not a blocker, but do think there any value in adding some test coverage for this?

👍 LGTM

Copy link

@bernabas bernabas left a comment

Choose a reason for hiding this comment

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

looks good

@philippschulte philippschulte force-pushed the pschulte/support-for-the-link-to-the-rule-set-deploy-status branch from 45903fb to f5fc665 Compare May 15, 2020 15:36
@philippschulte
Copy link
Member Author

@phamann I have added the missing tests. Thank you!

@philippschulte philippschulte merged commit ba2b0e0 into master May 15, 2020
@philippschulte philippschulte deleted the pschulte/support-for-the-link-to-the-rule-set-deploy-status branch May 15, 2020 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants