fix(diff): clear unmatching deprecated fields

[nowNick]

🟡 WIP 🛠️ 🟡

Depends on: Kong/go-kong#473

Summary

When user uses decK configuration and they use only new or the old (deprecated) fields then we don't want to display incorrect diff. We used to try to "fill" those fields but sometimes it's impossible to do that. The reason why we need to fill in these fields is that Kong, in order to be backwards compatible sends, both new and old fields. This commit removes either the deprecated or the new fields from the response from Kong before doing the diff so that there is no false negative difference.

Full changelog

• Implement ClearUnmatchingDeprecations
• Use this function in diff

Issues resolved

Fix KAG-5577

Documentation

• N/A Link to the website documentation PR

Testing

• Unit tests
• E2E tests
• Manual testing on Universal
• Manual testing on Kubernetes

[CLAassistant]

All committers have signed the CLA.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 29.26%. Comparing base (01fa2d0) to head (c146dc6).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/diff/diff.go	0.00%	8 Missing ⚠️
pkg/types/plugin.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
- Coverage   29.88%   29.26%   -0.63%     
==========================================
  Files         106      106              
  Lines       12631    15442    +2811     
==========================================
+ Hits         3775     4519     +744     
- Misses       8393    10460    +2067     
  Partials      463      463              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Prashansa-K]

@nowNick
I tested your changes on konnect as well as gateway 3.8 with the attached deck file. We do see some diffs still coming up. I feel some there are some clashes with the fill defaults code and the code you added in go-kong, that could be causing it. I am still trying to figure things out here, but attaching the file here as well so that you can also test this out.

_format_version: "3.0"
_info:
  defaults: {}
  select_tags:
  - netcracker
# _konnect:
#   control_plane_name: End2End
services:
- connect_timeout: 300000
  enabled: true
  host: nc-ee-toms.gci.com
  name: netcracker-ee-service
  plugins:
  - config:
      anonymous: null
      audience: null
      audience_claim:
      - aud
      audience_required: null
      auth_methods:
      - client_credentials
      - introspection
      authenticated_groups_claim: null
      authorization_cookie_domain: null
      authorization_cookie_http_only: true
      authorization_cookie_name: authorization
      authorization_cookie_path: /
      authorization_cookie_same_site: Default
      authorization_cookie_secure: null
      authorization_endpoint: null
      authorization_query_args_client: null
      authorization_query_args_names: null
      authorization_query_args_values: null
      authorization_rolling_timeout: 600
      bearer_token_cookie_name: null
      bearer_token_param_type:
      - header
      - query
      - body
      by_username_ignore_case: false
      cache_introspection: true
      cache_token_exchange: true
      cache_tokens: true
      cache_tokens_salt: 5jXqT82Wb7ITimAQ2bCrNwgGacOhVh7c
      cache_ttl: 3600
      cache_ttl_max: null
      cache_ttl_min: null
      cache_ttl_neg: null
      cache_ttl_resurrect: null
      cache_user_info: true
      client_alg: null
      client_arg: client_id
      client_auth: null
      client_credentials_param_type:
      - header
      - query
      - body
      client_id:
      - KongAdmin_EE
      client_jwk: null
      client_secret:
      - GQuYahXyRvsgLPIDOvphkCLZddKq7jseniZrjxpohefZHR8T6Jx2xFTa9c8nSYUt
      consumer_by:
      - username
      consumer_claim:
      - client_id
      consumer_optional: false
      credential_claim:
      - sub
      disable_session: null
      discovery_headers_names: null
      discovery_headers_values: null
      display_errors: true
      domains: null
      downstream_access_token_header: null
      downstream_access_token_jwk_header: null
      downstream_headers_claims: null
      downstream_headers_names: null
      downstream_id_token_header: null
      downstream_id_token_jwk_header: null
      downstream_introspection_header: null
      downstream_introspection_jwt_header: null
      downstream_refresh_token_header: null
      downstream_session_id_header: null
      downstream_user_info_header: null
      downstream_user_info_jwt_header: null
      dpop_proof_lifetime: 300
      dpop_use_nonce: false
      enable_hs_signatures: false
      end_session_endpoint: null
      expose_error_code: true
      extra_jwks_uris: null
      forbidden_destroy_session: true
      forbidden_error_message: Forbidden
      forbidden_redirect_uri: null
      groups_claim:
      - groups
      groups_required: null
      hide_credentials: true
      http_proxy: null
      http_proxy_authorization: null
      http_version: 1.1
      https_proxy: null
      https_proxy_authorization: null
      id_token_param_name: null
      id_token_param_type:
      - header
      - query
      - body
      ignore_signature: []
      introspect_jwt_tokens: false
      introspection_accept: application/json
      introspection_check_active: true
      introspection_endpoint: null
      introspection_endpoint_auth_method: null
      introspection_headers_client: null
      introspection_headers_names: null
      introspection_headers_values: null
      introspection_hint: access_token
      introspection_post_args_client: null
      introspection_post_args_names: null
      introspection_post_args_values: null
      introspection_token_param_name: token
      issuer: https://esso-ee.gci.com/.well-known/openid-configuration
      issuers_allowed: null
      jwt_session_claim: sid
      jwt_session_cookie: null
      keepalive: true
      leeway: 0
      login_action: upstream
      login_methods:
      - authorization_code
      login_redirect_mode: fragment
      login_redirect_uri: null
      login_tokens:
      - id_token
      logout_methods:
      - POST
      - DELETE
      logout_post_arg: null
      logout_query_arg: null
      logout_redirect_uri: null
      logout_revoke: false
      logout_revoke_access_token: true
      logout_revoke_refresh_token: true
      logout_uri_suffix: null
      max_age: null
      mtls_introspection_endpoint: null
      mtls_revocation_endpoint: null
      mtls_token_endpoint: null
      no_proxy: null
      password_param_type:
      - header
      - query
      - body
      preserve_query_args: false
      proof_of_possession_auth_methods_validation: true
      proof_of_possession_dpop: "off"
      proof_of_possession_mtls: "off"
      pushed_authorization_request_endpoint: null
      pushed_authorization_request_endpoint_auth_method: null
      redirect_uri: null
      rediscovery_lifetime: 30
      refresh_token_param_name: null
      refresh_token_param_type:
      - header
      - query
      - body
      refresh_tokens: true
      require_proof_key_for_code_exchange: null
      require_pushed_authorization_requests: null
      require_signed_request_object: null
      resolve_distributed_claims: false
      response_mode: query
      response_type:
      - code
      reverify: false
      revocation_endpoint: null
      revocation_endpoint_auth_method: null
      revocation_token_param_name: token
      roles_claim:
      - roles
      roles_required: null
      run_on_preflight: true
      scopes:
      - openid
      scopes_claim:
      - scope
      scopes_required: null
      search_user_info: false
      session_absolute_timeout: 86400
      session_audience: default
      session_compressor: none
      session_cookie_domain: null
      session_cookie_http_only: true
      session_cookie_maxsize: 4000
      session_cookie_name: session
      session_cookie_path: /
      session_cookie_renew: 600
      session_cookie_same_site: Lax
      session_cookie_secure: null
      session_enforce_same_subject: false
      session_hash_storage_key: false
      session_hash_subject: false
      session_idling_timeout: 900
      session_memcached_host: 127.0.0.1
      session_memcached_port: 11211
      session_memcached_prefix: sessions
      session_memcached_socket: null
      session_redis_cluster_max_redirections: null
      session_redis_cluster_nodes: null
      session_redis_connect_timeout: null
      session_redis_host: 127.0.0.1
      session_redis_password: null
      session_redis_port: 6379
      session_redis_prefix: sessions
      session_redis_read_timeout: null
      session_redis_send_timeout: null
      session_redis_server_name: null
      session_redis_socket: null
      session_redis_ssl: false
      session_redis_ssl_verify: false
      session_redis_username: null
      session_remember: false
      session_remember_absolute_timeout: 2592000
      session_remember_cookie_name: remember
      session_remember_rolling_timeout: 604800
      session_request_headers: null
      session_response_headers: null
      session_rolling_timeout: 3600
      session_secret: null
      session_storage: cookie
      session_store_metadata: false
      session_strategy: default
      ssl_verify: false
      timeout: 10000
      tls_client_auth_cert_id: null
      tls_client_auth_ssl_verify: true
      token_cache_key_include_scope: false
      token_endpoint: null
      token_endpoint_auth_method: null
      token_exchange_endpoint: null
      token_headers_client: null
      token_headers_grants: null
      token_headers_names: null
      token_headers_prefix: null
      token_headers_replay: null
      token_headers_values: null
      token_post_args_client: null
      token_post_args_names: null
      token_post_args_values: null
      unauthorized_destroy_session: true
      unauthorized_error_message: Unauthorized
      unauthorized_redirect_uri: null
      unexpected_redirect_uri: null
      upstream_access_token_header: authorization:bearer
      upstream_access_token_jwk_header: null
      upstream_headers_claims: null
      upstream_headers_names: null
      upstream_id_token_header: null
      upstream_id_token_jwk_header: null
      upstream_introspection_header: null
      upstream_introspection_jwt_header: null
      upstream_refresh_token_header: null
      upstream_session_id_header: null
      upstream_user_info_header: null
      upstream_user_info_jwt_header: null
      userinfo_accept: application/json
      userinfo_endpoint: null
      userinfo_headers_client: null
      userinfo_headers_names: null
      userinfo_headers_values: null
      userinfo_query_args_client: null
      userinfo_query_args_names: null
      userinfo_query_args_values: null
      using_pseudo_issuer: false
      verify_claims: true
      verify_nonce: true
      verify_parameters: false
      verify_signature: true
    enabled: true
    name: openid-connect
    protocols:
    - grpc
    - grpcs
    - http
    - https
  port: 443
  protocol: https
  read_timeout: 300000
  retries: 5
  routes:
  - hosts:
    - api-ee.gci.com
    https_redirect_status_code: 426
    name: netcracker-ee-route
    path_handling: v0
    paths:
    - /vendor/v1/netcracker
    preserve_host: false
    protocols:
    - http
    - https
    regex_priority: 0
    request_buffering: true
    response_buffering: true
    strip_path: true
  write_timeout: 300000

Few diffs still come up.

1. Some fields that don't have translate_backwards or replaced_with show up, which seems understandable for now.

+    "session_compressor": "none"
+    "session_cookie_maxsize": 4000
+    "session_cookie_renew": 600
+    "session_strategy": "default"

2. Some fields show diff each time we run a sync/diff.
   Redis configs:

-      "read_timeout": null,
+      "read_timeout": 2000,
-      "send_timeout": null,
+      "send_timeout": 2000,

3. Some diffs have started coming up that I think were not coming up earlier.
   Redis configs:

-       "timeout": null
+      "timeout": 2000
+      "cluster_nodes": null
+      "password": null
+      "sentinel_nodes": null
+      "server_name": null
+      "socket": null
+      "username": null

[nowNick]

Ok @Prashansa-K I think I know what's wrong. The point 2 & 3 are the case when a new field contains a default value. For example - in the config you presented there's session_redis_read_timeout: null. It is a deprecated field and in the new field redis.read_timeout should be used in it's place.... However 😓 ... what decK does is:

1. It pulls plugin schema
2. It sees the the configuration in the yaml file does not contain certain fields.
3. Based on the schema it tries to fill them and inserts redis.read_timeout = 2000 because the new field has defined a default
4. Compares the generated conifguration with the one returned by Kong. Kong says that session_redis_read_timeout = null and redis.read_timeout = null (which is correct). However decK sees that it's configuration contains: session_redis_read_timeout = null but the new field redis.read_timeout = 2000 and it shows a diff.

I think the solution here is to improve the default filling logic. Right now we base it on what's in the yaml file and what's in the schema. It fills the missing fields with null or default value if one is defined.
What I think we should be doing is to also check if the value that's about to be filled might be defined in it's deprecated version. If that's the case then we shouldn't fill it.

It's getting more and more pear shaped. I'm happy to explore other / simpler solutions. However I'm afraid that still - what we used to do - rewrite the value from deprecated field to new field won't cut it anymore.