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

Add flag to skip CA certificates #2341

Merged
merged 1 commit into from
May 2, 2022
Merged

Add flag to skip CA certificates #2341

merged 1 commit into from
May 2, 2022

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Mar 18, 2022

What this PR does / why we need it:
This adds a flag to control the deck behavior added in Kong/deck#617

Special notes for your reviewer:

  • At present, deck doesn't provide an option to skip these in file parsing (i.e. generating the target config), so it and this draft strip the certificates out after the fact. We should add an option to the file.RenderConfig to handle this instead so that the controller doesn't duplicate logic. chore(file) refactor target config cert omission deck#622 changes this and the draft has been updated to remove the duplicated logic.
  • This currently uses an unreleased deck tag.

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@rainest rainest temporarily deployed to Configure ci March 18, 2022 18:55 Inactive
@rainest rainest temporarily deployed to Configure ci March 18, 2022 18:55 Inactive
@rainest rainest temporarily deployed to Configure ci March 18, 2022 18:57 Inactive
@rainest rainest temporarily deployed to Configure ci March 18, 2022 18:57 Inactive
@rainest
Copy link
Contributor Author

rainest commented Mar 18, 2022

Manual testing since this a flag that doesn't really work well with our normal test environment (basically all disable X flags are like this):

logs logs logs ====
...
creating ca-certificate cce8c384-721f-4f58-85dd-50834e3e733a
time="2022-03-18T18:44:52Z" level=info msg="successfully synced configuration to kong."

11:45:01-0700 esenin $ curl -ks https://localhost:8999/ca_certificates | jq
{                                                      
  "data": [
    {
      "cert_digest": "e60e48c8a64f9bfeb0402bbf6da22417c98cd8263c7bdaf84bb510d59b2c3c89",
      "id": "cce8c384-721f-4f58-85dd-50834e3e733a",
      "created_at": 1647629092,
      "cert": "-----BEGIN CERTIFICATE-----\nMIIDazCCAlOgAwIBAgIUQyMJLbNDvnpT5pKY24PADnJi3i0wDQYJKoZIhvcNAQEL\nBQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM\nGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMjAzMTgxODE0NDVaFw0zMjAz\nMTUxODE0NDVaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw\nHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3DQEB\nAQUAA4IBDwAwggEKAoIBAQDXHKNUZYbMkYbuqGPEBmFh9+QuLLtNFmRfuZgFPD3Q\nI8Aw0zPjJgOZXMRwRyzM/7DAqG99lyzigBadZ0dbVoPP86+4xFRccDl8Oat0KoAm\nPAyMLb2fMpBkVJ0PVq0iaXjtlpwrO/cf1Co0pQ9wARkBhAFjRC/YYOZQJGrmay3s\nQudmLp8l6CQmyeNEa1awXKpupANMd/EOsbcYH1PsMk5fr9vCdhHQVAkZZA9HLTy6\nDAte9O9pzpaE4S7SYN80KfuYu89rY4gxla5COKlCsm9u1yr2TOnOSyDzhqGWQi6r\n6bkfcjFr3qEM4N1xD1EvtFEc4L/4GXv2DoQG3wflE4eRAgMBAAGjUzBRMB0GA1Ud\nDgQWBBRLFscPBGM/5vo4t9XriwuAPXCPvDAfBgNVHSMEGDAWgBRLFscPBGM/5vo4\nT9XriwuAPXCPvDAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAP\nRprnkC4vpPsPq3/NmRpO/88U04/mUm9xwbocGBRnbySeh4tf39txV/XwI5D4HJC8\nO4dguiwTOkMZ2WWRkS3mxxdPPphBSKSxHMts/F2cyXO+nNns8PrXZ8gaOswyguuV\nVAvRQ/fJW9MTkY++M6nl9ATAgUcb1JB7pmdCxATm+7TRIiDcAKxbwGW1V49KGLsh\nMtXJVWMvD5scJQAK2kqlX42EVL4U+F65W6XwAnlsNODhYG8CSRYPN6zXtEtmw5k0\n0tJiho2M1w/040zNcKHONhCY33BEqomPx2ODwb5K89XJuJdvZecsJl8g3GX4U0Pu\nES2z8EsGaMmuNEwJkks/\n-----END CERTIFICATE-----\n",
      "tags": [
        "managed-by-ingress-controller"
      ]
    }
  ],
  "next": null
}

11:45:03-0700 esenin $ kubectl get deployments.apps aaa-kong -oyaml | grep -A1 DISABLE_CA

11:45:27-0700 esenin $ kubectl edit deployments.apps aaa-kong
deployment.apps/aaa-kong edited

11:45:53-0700 esenin $ kubectl get deployments.apps aaa-kong -oyaml | grep -A1 DISABLE_CA
        - name: CONTROLLER_DISABLE_CA_CERTIFICATES
          value: "true"

11:47:11-0700 esenin $ curl -ks https://localhost:8999/ca_certificates | jq              
{
  "next": null,
  "data": [
    {
      "id": "cce8c384-721f-4f58-85dd-50834e3e733a",
      "cert": "-----BEGIN CERTIFICATE-----\nMIIDazCCAlOgAwIBAgIUQyMJLbNDvnpT5pKY24PADnJi3i0wDQYJKoZIhvcNAQEL\nBQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM\nGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMjAzMTgxODE0NDVaFw0zMjAz\nMTUxODE0NDVaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw\nHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3DQEB\nAQUAA4IBDwAwggEKAoIBAQDXHKNUZYbMkYbuqGPEBmFh9+QuLLtNFmRfuZgFPD3Q\nI8Aw0zPjJgOZXMRwRyzM/7DAqG99lyzigBadZ0dbVoPP86+4xFRccDl8Oat0KoAm\nPAyMLb2fMpBkVJ0PVq0iaXjtlpwrO/cf1Co0pQ9wARkBhAFjRC/YYOZQJGrmay3s\nQudmLp8l6CQmyeNEa1awXKpupANMd/EOsbcYH1PsMk5fr9vCdhHQVAkZZA9HLTy6\nDAte9O9pzpaE4S7SYN80KfuYu89rY4gxla5COKlCsm9u1yr2TOnOSyDzhqGWQi6r\n6bkfcjFr3qEM4N1xD1EvtFEc4L/4GXv2DoQG3wflE4eRAgMBAAGjUzBRMB0GA1Ud\nDgQWBBRLFscPBGM/5vo4t9XriwuAPXCPvDAfBgNVHSMEGDAWgBRLFscPBGM/5vo4\nT9XriwuAPXCPvDAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAP\nRprnkC4vpPsPq3/NmRpO/88U04/mUm9xwbocGBRnbySeh4tf39txV/XwI5D4HJC8\nO4dguiwTOkMZ2WWRkS3mxxdPPphBSKSxHMts/F2cyXO+nNns8PrXZ8gaOswyguuV\nVAvRQ/fJW9MTkY++M6nl9ATAgUcb1JB7pmdCxATm+7TRIiDcAKxbwGW1V49KGLsh\nMtXJVWMvD5scJQAK2kqlX42EVL4U+F65W6XwAnlsNODhYG8CSRYPN6zXtEtmw5k0\n0tJiho2M1w/040zNcKHONhCY33BEqomPx2ODwb5K89XJuJdvZecsJl8g3GX4U0Pu\nES2z8EsGaMmuNEwJkks/\n-----END CERTIFICATE-----\n",
      "cert_digest": "e60e48c8a64f9bfeb0402bbf6da22417c98cd8263c7bdaf84bb510d59b2c3c89",
      "tags": [
        "managed-by-ingress-controller"
      ],
      "created_at": 1647629092
    }
  ]
}

Oh no! That's not what we wanted at all! A caveat about this is that it's making the controller blind to certificates, so if you'd already created one it'll sit around in the DB. Flipping tthe flag will not remove existing certificates, but they won't come back if you delete and restart to force a fresh sync:

12:07:32-0700 esenin $ curl -ksX DELETE https://localhost:8999/ca_certificates/cce8c384-721f-4f58-85dd-50834e3e733a   

12:07:50-0700 esenin $ curl -ks https://localhost:8999/ca_certificates | jq
{                                                                          
  "data": [],
  "next": null
}

12:07:55-0700 esenin $ kubectl rollout restart deployment aaa-kong 
deployment.apps/aaa-kong restarted

12:09:29-0700 esenin $ curl -ks https://localhost:8999/ca_certificates | jq
{
  "data": [],
  "next": null
}

@rainest rainest temporarily deployed to Configure ci March 18, 2022 19:15 Inactive
@rainest rainest temporarily deployed to Configure ci March 18, 2022 19:52 Inactive
@rainest rainest temporarily deployed to Configure ci March 18, 2022 19:52 Inactive
@rainest rainest temporarily deployed to Configure ci March 18, 2022 20:07 Inactive
@rainest rainest temporarily deployed to Configure ci March 18, 2022 20:07 Inactive
@rainest rainest temporarily deployed to Configure ci March 18, 2022 20:27 Inactive
@mflendrich
Copy link
Contributor

Blocked on the release of decK providing this feature.

@rainest rainest removed the blocked label Apr 25, 2022
@rainest rainest marked this pull request as ready for review April 25, 2022 18:16
@rainest rainest requested a review from a team as a code owner April 25, 2022 18:16
@rainest rainest temporarily deployed to Configure ci April 25, 2022 18:16 Inactive
@rainest rainest temporarily deployed to Configure ci April 25, 2022 18:17 Inactive
@rainest rainest temporarily deployed to Configure ci April 25, 2022 18:22 Inactive
@rainest rainest temporarily deployed to Configure ci April 25, 2022 18:22 Inactive
@rainest rainest temporarily deployed to Configure ci April 25, 2022 18:42 Inactive
@rainest rainest merged commit ff4ca0c into main May 2, 2022
@rainest rainest deleted the feat/skip-ca branch May 2, 2022 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants