chore(lib): Update cert_handler library to v1 #517

rgildein · 2024-08-16T13:52:54Z

Update the cert_handler to v1.

fixes: #516

Update the cert_handler to v1. fixes: #516

DnPlas

Thanks @rgildein ! Very few details, other than that LGTM.

I have tested this by:

Deploying istio-operators from this PR
Checking the Gateway just right after (1) to verify no TLS config is present

$ kubectl get gateway -nkubeflow istio-gateway -oyaml
apiVersion: networking.istio.io/v1
kind: Gateway
metadata:
  creationTimestamp: "2024-08-21T09:06:27Z"
  generation: 1
  labels:
    app.juju.is/created-by: istio-pilot
    app.kubernetes.io/instance: istio-pilot-kubeflow
    kubernetes-resource-handler-scope: gateway
  name: istio-gateway
  namespace: kubeflow
  resourceVersion: "82649"
  uid: a159ce23-70f5-4ff0-8141-79d6f867a33a
spec:
  selector:
    istio: ingressgateway
  servers:
  - hosts:
    - '*'
    port:
      name: http
      number: 8080
      protocol: HTTP

Deployed self-signed-certs and integrate

$ juju deploy self-signed-certificates --channel latest/edge --trust
$ juju relate istio-pilot:certificates self-signed-certificates:certificates

Verify TLS is configured in the Gateway:

kubectl get gateways -nkubeflow istio-gateway -oyaml
apiVersion: networking.istio.io/v1
kind: Gateway
metadata:
  creationTimestamp: "2024-08-21T09:06:27Z"
  generation: 2
  labels:
    app.juju.is/created-by: istio-pilot
    app.kubernetes.io/instance: istio-pilot-kubeflow
    kubernetes-resource-handler-scope: gateway
  name: istio-gateway
  namespace: kubeflow
  resourceVersion: "83154"
  uid: a159ce23-70f5-4ff0-8141-79d6f867a33a
spec:
  selector:
    istio: ingressgateway
  servers:
  - hosts:
    - '*'
    port:
      name: https # <--- this should be https
      number: 8443
      protocol: HTTPS
    tls: <--- this should exist
      credentialName: istio-gateway-gateway-secret <--- this should point to an existing secret
      mode: SIMPLE

# Check the secret has tls.crt and tls.key values
$ kubectl get secret -nkubeflow istio-gateway-gateway-secret -oyaml

apiVersion: v1
data:
  tls.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURuekNDQW9lZ0F3SUJBZ0lVTS9iUDBUdW51RCtnTlloSGllaEtYK0Yya0NZd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0xERXFNQ2dHQTFVRUF3d2hjMlZzWmkxemFXZHVaV1F0WTJWeWRHbG1hV05oZEdWekxXOXdaWEpoZEc5eQpNQjRYRFRJME1EZ3lNVEE1TURnek5Wb1hEVEkxTURneU1UQTVNRGd6TlZvd1JqRVZNQk1HQTFVRUF3d01NVEF1Ck5qUXVNVFF3TGpRek1TMHdLd1lEVlFRdERDUmtNREUyWlRCaE15MHlNVEk1TFRSaFltVXRZakZqWXkweU16a3gKWmpobFpUZ3hOamN3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRQ3p2QkZQU29LSAp5cVN5a1RUWDFqalR0Nnkwcm9oOUJabmt5azVYcjVGNXZOdUswd2k5aTE5S0tnTjZDdlplTjJuU3VsYmphYldkCjJUWWVtWnhSM0ZTenRjS1dRc0wzaVhadVg4YkVZQVljQnFzMVZtL3I4YXhJR01BWCt4bE91TTVFZHNTR2VVNmsKVzk3Y0NYK3BmTnp3SWNLcXBIUmtVTGVBSmdRaU5EV1ZJV1ZRalh1UXlGK0F4bkJNSGFXa2ZTemRYSDB1dkRTOQoxLzBCa0x4N3d0cXhZRkxqTlk4RGpJUnpMTmRwY1Z0L0xmL0tscmtDTU5iWmRVYjM4UlRRSkM0Vm5rQUI3U2dlCkxkQzIyZ2NiMmlLWTFLY201MmQxTFdVYUxMQUhhYUlUYTNwTU1NSk1OR28yb3JjVTIrNUZLTGtDVmNnTjc4MUIKbnB1YUp1eW9LUXZQQWdNQkFBR2pnWjR3Z1pzd0lRWURWUjBqQkJvd0dJQVdCQlNKcjVWdEljZ3lHUEFtSURZTwp4UlJKbktTUjF6QWRCZ05WSFE0RUZnUVV2T3NMb1dZSGtNY1IxalAxVDVHOWluK0w4cjh3REFZRFZSMFRBUUgvCkJBSXdBREJKQmdOVkhSRUVRakJBZ2o1cGMzUnBieTF3YVd4dmRDMHdMbWx6ZEdsdkxYQnBiRzkwTFdWdVpIQnYKYVc1MGN5NXJkV0psWm14dmR5NXpkbU11WTJ4MWMzUmxjaTVzYjJOaGJEQU5CZ2txaGtpRzl3MEJBUXNGQUFPQwpBUUVBZ0lmRDdCMHl2MjJoMnBvUm5QYlE4K2Z3b0lIa3NZSzREcCtwNDJ1ZC9EdjAxU3A2NHNsSE51a2FCZFkvCnNNYlVSSnFTejU2c05qeUZTYkRGY3UrVXNWOFhJdklxMVYvZHFYbnZyQWpoRThTeVhueW91Q29uYkhGL05CcWIKczR0MlZROVdqdmgrVTZWTi9VWlh3K1B2bml2TStpQ0tjckRKZmxpaXpibVI5bm91aDN4TDMzNHlpTHNuLzFybAppQjlYa1FQejU2bDdubFkyd1VKc2hiWWJzVEtMQ2x0djBmSGUzWWFMSTE5dnQ0Umdsd1hkOUloQm1vNlB6aURVCit5cHhOR3BWYUlQQk93cHQvay9QQWs4S2Vackg2bTFQVklQR0NKVWNpYWo0NU9RRUtzRXp3TmRybjFOSVUxdjQKcUNRMmJnZzR1Y1MvRnFtTFpVcTU0RWVsY0E9PQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0t
  tls.key: LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlFb3dJQkFBS0NBUUVBczd3UlQwcUNoOHFrc3BFMDE5WTQwN2VzdEs2SWZRV1o1TXBPVjYrUmViemJpdE1JCnZZdGZTaW9EZWdyMlhqZHAwcnBXNDJtMW5kazJIcG1jVWR4VXM3WENsa0xDOTRsMmJsL0d4R0FHSEFhck5WWnYKNi9Hc1NCakFGL3NaVHJqT1JIYkVobmxPcEZ2ZTNBbC9xWHpjOENIQ3FxUjBaRkMzZ0NZRUlqUTFsU0ZsVUkxNwprTWhmZ01ad1RCMmxwSDBzM1Z4OUxydzB2ZGY5QVpDOGU4TGFzV0JTNHpXUEE0eUVjeXpYYVhGYmZ5My95cGE1CkFqRFcyWFZHOS9FVTBDUXVGWjVBQWUwb0hpM1F0dG9IRzlvaW1OU25KdWRuZFMxbEdpeXdCMm1pRTJ0NlREREMKVERScU5xSzNGTnZ1UlNpNUFsWElEZS9OUVo2Ym1pYnNxQ2tMendJREFRQUJBb0lCQUVpak93T21nelpKNldIWgpXVmZaVmNJS3V4dVNaY3JSRnE3bUs5ODRMenpaM0lnd1habnMxNmZyYnRoRjBlZWwwWGkrb2hycVArSDVST3Y4Ci9MWUFxNkt0VkdUUnVtVzhBa2I5SWlGL0JUa1NZT0wvZWVBTEhhdE5oV1Nyc0VDbVk0WTcwWlRmTmE4ckNkZzMKWm9haTFjK2VkVVB0anJSMEFwVWh5QTNpdDd6Nndzcm5NZysyb0VjeWxoSGFoUEM0VEp2TXVVU2lDWW9xcnB1eApEd1Fmck9rWVZFK3VJYXF0TGJQcHUvWTFOOGxmK1VjNEQ4Ny95T0pQRlZPOFB6bjZ0ZTIySlUwTjB5dXdyekg5CnNRN1pFREhkOGxTUzJNc2pSWFRrUGZYUG93Qmg2UGZ5SnhsNGlXNXlqY0wrVUpqclpKRkswVVdOcFRVMHVuMSsKR1d6Z2RIRUNnWUVBMTlGQ3dTZ1RVN21KRVpNbFJ6TGZLNzd0SjJTQ24yNEd5QjJsdFJVOEZUSTFmU0ZZQlh1ZApNdUt6WjcwRjQ1aDJHZXowZ010TFZ4dGpML0NiSGhaR3FqNFJuRmozTk9tSmloSWlxZUI2aU1VSExCRjJhSWdSCng4Z2hTYjNRU21Oa1lTVXRVMnlMU3JkanpSTUFTcE1GYjFsVm1ueTM5K1llWmc2V0g1TmM5RFVDZ1lFQTFUTDAKTzJwRU1TQ2VKZDJzaTVIVWQxaWJ0VzBzSUpIeVFVd1RtUWRRazZkdzZsMHZkOXJNYkN3cjRiQW9ta1FXMkdaZQpOQXhMZ25lY0R2K1drL0ZPbnhOREV6Ny9NQ1E3S2pocVpjQnRaS09ORzFVYk1SY1lGeTVybytVUUFlcDV5NDh0CldoUEMyQVZkT1JnNUlNRlo5ZTVkdkFONWZrMUdHbXYwTERnRStITUNnWUVBcDkwbFhoWXN5aitTeEsrK0hCNE8KaGZrd2Z5Wm5qMWhHUUJzSFM4MGplWjBmQzZBRzFlVHJSYXdkUFVCQ04xL2I1Smh4S1VoMjVsN3dERmJLWUdHVgpQMCtkNVEweDR0OFBVdXgrTjhIWnJVNExJUlRJRTlCYWZCbEhBeE4zMHBSeWZEa3RneWozUXZ0WHppZk1YelR4CjBrVWJGMW1Rd21va0ZOK2RseHZJL2swQ2dZQWZlOHpSVVZvTW56SjdpUWJIL1pzUW5NY3h2Wk44bzlEUWo3bDkKS2JWZWVLV1dGbmpDREUrUDBkNFJFQUNPOTJzZ1BjMi9oZWxJdFAwWXdlbXNvei9uQWVNdjNtZTA1a1RPY1ZKVgpBRnVuTnZmSmg0SGlkL1NZeDhRaGlkd1pURlQ4R0lLc0FLc1BWNHR5dVA4R3RVYmhxSGV6SWhnNDdKUmpwbm1DClppdGx2UUtCZ0hFUXJpRnV5bHVaeDNxUm1rNU9yYVJrL3NPT3NsTXVzNzRWTG1wSTNzUFIrcDlsUlN6d3JsRVoKc2RxT2tyS2swcVR2ZDA4TmlvYUdVdHp2UHAxYVgwTDJ5eGl1NkYwZnJ1eitQaVpmUG5BNlNNYTdmOWtaVS9lTQpnQzg4bExqNWg4SzlDTXJpaVpqWFRWTmN1SW5IVnMxSG15bnIvYmdENDBjdEhkTHVYS2JVCi0tLS0tRU5EIFJTQSBQUklWQVRFIEtFWS0tLS0tCg==
kind: Secret
metadata:
  creationTimestamp: "2024-08-21T09:08:36Z"
  labels:
    app.juju.is/created-by: istio-pilot
    app.kubernetes.io/instance: istio-pilot-kubeflow
    kubernetes-resource-handler-scope: gateway
  name: istio-gateway-gateway-secret
  namespace: kubeflow
  resourceVersion: "83152"
  uid: c772f2f2-db16-44b5-8415-a843fd61c8a9
type: kubernetes.io/tls

.github/workflows/integrate.yaml

DnPlas · 2024-08-21T09:03:10Z

charms/istio-pilot/src/charm.py

@@ -244,7 +243,7 @@ def _istioctl_extra_flags(self):
                    "--set",
                    f"values.cni.cniConfDir={self.model.config['cni-conf-dir']}",
                    "--set",
-                    "values.sidecarInjectorWebhook.injectedAnnotations.traffic\.sidecar\.istio\.io/excludeOutboundIPRanges=0.0.0.0/0",  # noqa
+                    "values.sidecarInjectorWebhook.injectedAnnotations.traffic\\.sidecar\\.istio\\.io/excludeOutboundIPRanges=0.0.0.0/0",  # noqa


What's with the double \\?

linting issue and list locally the linting was failing for me. In Python the \ can be put before a special character, e.g. print("\\") -> \.

It is weird the linting was failing for you, this line should be skipped actually. Not a blocker, but preferably it would be in a different PR.

I tried it again, and it was not failing, but I saw warning (failure must due something else).

lint: commands[1]> pflake8 /home/rgildein/code/canonical/istio-operators/charms/istio-pilot/src/ /home/rgildein/code/canonical/istio-operators/charms/istio-pilot/tests/ /home/rgildein/code/canonical/istio-operators/charms/istio-pilot/lib/charms/istio_pilot <unknown>:246: SyntaxWarning: invalid escape sequence '\.'

Reverting this change.

DnPlas · 2024-08-21T09:04:24Z

tests/test_bundle.py

@@ -275,7 +275,7 @@ async def test_enable_ingress_auth(ops_test: OpsTest):
    await ops_test.model.wait_for_idle(
        status="active",
        raise_on_blocked=False,
-        timeout=90 * 10,
+        timeout=60 * 20,


Let's not change the timeout, it was ruled out as not being the cause of failure of the CI, right?

That's right, we can keep it. Do you think it's ok if I use 60 * 15? Same timeout but in more readable format, 15 minutes.

Sounds good.

DnPlas · 2024-08-21T09:05:19Z

tests/test_bundle_tls_provider.py

@@ -27,6 +27,7 @@ def lightkube_client() -> lightkube.Client:


 @pytest.mark.abort_on_fail
+@pytest.mark.skip_if_deployed


Usually we don't use @pytest.mark.skip_if_deployed in the CI, as nothing should be deployed beforehand, otherwise this can be considered an error. Did you leave them by mistake?

This is used only if in pytest-operator you call testing with --no-deploy, so it's really useful if you want to run test multiple time locally, as I was doing. We can remove it if you think so, but I do not see any harm to leave it there.

I think that a lot of CKF tests using decorator pytest.mark.abort_on_fail wrongly, since it's meant to be used only once for part which is responsible for deployment. At the same time it should be used with skip_if_deployed decorator so you can easily run tests multiple time (with --no-deploy) without need to run test from scratch (create new model, build charm, deploy it and others, etc.). For example I do not understand why test below test_tls_configuration, is marked with abort_on_fail? It's only one test.

Would it make sense then to have a small refactoring PR where we remove the decorators that do not make sense and add the ones that can help the dev experience a bit better?

I can definitely see the value of @pytest.mark.skip_if_deployed when running the tests locally, despite this test file being used mostly in the CI.

That sounds good, I will report an issue and later open the PR. Reverting these changes for now.

Signed-off-by: Robert Gildein <[email protected]>

DnPlas

Thanks @rgildein ! This works for me and it was a really needed upgrade! Great job!

Update the cert_handler to v1. fixes: #516 --------- Signed-off-by: Robert Gildein <[email protected]>

chore(lib): Update cert_handler library to v1

ce62a9c

Update the cert_handler to v1. fixes: #516

rgildein added the bug Something isn't working label Aug 16, 2024

rgildein self-assigned this Aug 16, 2024

rgildein requested a review from a team as a code owner August 16, 2024 13:52

github-actions bot added Libraries: OK Libraries: Out of sync and removed Libraries: OK Libraries: Out of sync labels Aug 16, 2024

increase timeout

a3440a5

rgildein force-pushed the bug/KF-6129/update-lib branch from 205d1cc to a3440a5 Compare August 19, 2024 10:22

github-actions bot added Libraries: OK Libraries: Out of sync and removed Libraries: Out of sync Libraries: OK labels Aug 19, 2024

rgildein force-pushed the bug/KF-6129/update-lib branch from 432eeec to 11d813e Compare August 20, 2024 09:15

Clean up runner to free some resource

2bbc124

rgildein force-pushed the bug/KF-6129/update-lib branch from 11d813e to 2bbc124 Compare August 20, 2024 09:16

github-actions bot added Libraries: OK Libraries: Out of sync and removed Libraries: Out of sync Libraries: OK labels Aug 20, 2024

DnPlas reviewed Aug 21, 2024

View reviewed changes

rgildein requested a review from DnPlas August 21, 2024 11:51

revert clean up action

f914bac

Signed-off-by: Robert Gildein <[email protected]>

rgildein changed the base branch from main to dev/KF-6129 August 21, 2024 12:11

Merge branch 'dev/KF-6129' into bug/KF-6129/update-lib

6a48a43

github-actions bot added Libraries: OK Libraries: Out of sync and removed Libraries: Out of sync Libraries: OK labels Aug 21, 2024

apply suggestions

d6c9e0c

rgildein force-pushed the bug/KF-6129/update-lib branch from 6cd01de to d6c9e0c Compare August 22, 2024 08:02

github-actions bot added Libraries: OK Libraries: Out of sync and removed Libraries: Out of sync Libraries: OK labels Aug 22, 2024

DnPlas approved these changes Aug 22, 2024

View reviewed changes

rgildein merged commit 5279b0c into dev/KF-6129 Aug 22, 2024
19 checks passed

rgildein deleted the bug/KF-6129/update-lib branch August 22, 2024 09:17

rgildein added a commit that referenced this pull request Aug 22, 2024

chore(lib): Update cert_handler library to v1 (#517)

738d5fa

Update the cert_handler to v1. fixes: #516 --------- Signed-off-by: Robert Gildein <[email protected]>

rgildein added a commit that referenced this pull request Aug 22, 2024

chore(lib): Update cert_handler library to v1 (#517)

d79512d

Update the cert_handler to v1. fixes: #516 --------- Signed-off-by: Robert Gildein <[email protected]>

rgildein added a commit that referenced this pull request Aug 22, 2024

chore(lib): Update cert_handler library to v1 (#517)

2c3e459

Update the cert_handler to v1. fixes: #516 --------- Signed-off-by: Robert Gildein <[email protected]>

rgildein added a commit that referenced this pull request Aug 22, 2024

chore(lib): Update cert_handler library to v1 (#517)

3e8dc3c

Update the cert_handler to v1. fixes: #516 --------- Signed-off-by: Robert Gildein <[email protected]>

rgildein added a commit that referenced this pull request Aug 22, 2024

chore(lib): Update cert_handler library to v1 (#517)

15c38f2

Update the cert_handler to v1. fixes: #516 --------- Signed-off-by: Robert Gildein <[email protected]>

orfeas-k pushed a commit that referenced this pull request Aug 26, 2024

chore(lib): Update cert_handler library to v1 (#517)

f7bcb80

Update the cert_handler to v1. fixes: #516 --------- Signed-off-by: Robert Gildein <[email protected]>

orfeas-k mentioned this pull request Aug 26, 2024

fix(ci): Merge kf-5656-dev-branch in track/1.17 #531

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(lib): Update cert_handler library to v1 #517

chore(lib): Update cert_handler library to v1 #517

rgildein commented Aug 16, 2024

DnPlas left a comment •

edited

Loading

DnPlas Aug 21, 2024

rgildein Aug 21, 2024

DnPlas Aug 22, 2024

rgildein Aug 22, 2024

DnPlas Aug 21, 2024

rgildein Aug 21, 2024

DnPlas Aug 22, 2024

DnPlas Aug 21, 2024

rgildein Aug 21, 2024

DnPlas Aug 22, 2024

rgildein Aug 22, 2024

DnPlas left a comment

		@@ -27,6 +27,7 @@ def lightkube_client() -> lightkube.Client:


		@pytest.mark.abort_on_fail
		@pytest.mark.skip_if_deployed

chore(lib): Update cert_handler library to v1 #517

chore(lib): Update cert_handler library to v1 #517

Conversation

rgildein commented Aug 16, 2024

DnPlas left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DnPlas left a comment

Choose a reason for hiding this comment

DnPlas left a comment •

edited

Loading