THREESCALE-9558: Separate env vars for Core API and public route #821

mayorova · 2023-04-24T11:25:29Z

The description about different Backend URLs that are used in the System component are provided in the porta PR description.

This PR removes the BACKEND_ROUTE environment variable, whose value is now http://backend-listener:3000/internal/ - which is confusing because "route" kind of suggests that it would be the value of the backend OCP route.

What

Deleted system-app env var APICAST_BACKEND_ROOT_ENDPOINT
Deleted system-app env var BACKEND_ROUTE
Add system-app env var BACKEND_URL: service_endpoint value from backend-listener secret (i.e. http://backend-listener:3000). Note /internal is no longer provided. That will be added by System.
Add system-app env var BACKEND_PUBLIC_URL: route_endpoint value from backend-listener secret (i.e. https://backend-3scale.apps.mycluster.com).
Upgrade procedure from the previous release

Verification steps

Requires openshift (oc CLI) session

New deployment

Create new project

oc new-project 3scale-testing-new-deployment

Checkout this branch and run the operator

make install
make run

Deploy 3scale
- no need to specify S3 credentials
- wildcardDomain must be something valid (only if it is going to be used later). Usually ${PROJECT_NAME}.apps.${CLUSTER_DOMAIN}

k apply -f - <<EOF
apiVersion: v1
kind: Secret
metadata:
  creationTimestamp: null
  name: aws-auth
stringData:
  AWS_ACCESS_KEY_ID: testID
  AWS_SECRET_ACCESS_KEY: testkey
  AWS_BUCKET: testbucket
  AWS_REGION: us-east-1
type: Opaque
EOF

k apply -f - <<EOF
---
apiVersion: apps.3scale.net/v1alpha1
kind: APIManager
metadata:
  name: apimanager1
spec:
  wildcardDomain: test01.example.net
  resourceRequirementsEnabled: false
  system:
    fileStorage:
      simpleStorageService:
        configurationSecretRef:
          name: aws-auth
EOF

wait for deployment to be ready

oc wait --for=condition=available apimanager/apimanager1 --timeout=-1s
apimanager.apps.3scale.net/apimanager1 condition met

The above "wait" ensures the pods are up and running.

Check the system-app deployment no longer has APICAST_BACKEND_ROOT_ENDPOINT or BACKEND_ROUTE env vars

# It should report empty
❯ k get dc system-app -o yaml | grep -A 1  APICAST_BACKEND_ROOT_ENDPOINT

# It should report empty
❯ k get dc system-app -o yaml | grep -A 1 BACKEND_ROUTE

Check the system-app deployment has the BACKEND_URL env var from the backend-listener secret and service_endpoint field

❯ k get dc system-app -o yaml | grep -A 4 BACKEND_URL
          - name: BACKEND_URL
            valueFrom:
              secretKeyRef:
                key: service_endpoint
                name: backend-listener
--
        - name: BACKEND_URL
          valueFrom:
            secretKeyRef:
              key: service_endpoint
              name: backend-listener
--
        - name: BACKEND_URL
          valueFrom:
            secretKeyRef:
              key: service_endpoint
              name: backend-listener
--
        - name: BACKEND_URL
          valueFrom:
            secretKeyRef:
              key: service_endpoint
              name: backend-listener

It is replicated 4 times in 4 different deployment: pre-hook pod deployment container and system-master, system-provider, system-developer containers.

Check the system-app deployment has the BACKEND_PUBLIC_URL env var from the backend-listener secret and route_endpoint field

❯ k get dc system-app -o yaml | grep -A 4 BACKEND_PUBLIC_URL
          - name: BACKEND_PUBLIC_URL
            valueFrom:
              secretKeyRef:
                key: route_endpoint
                name: backend-listener
--
        - name: BACKEND_PUBLIC_URL
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener
--
        - name: BACKEND_PUBLIC_URL
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener
--
        - name: BACKEND_PUBLIC_URL
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener

The backend-listener secret values

❯ kubectl get secret backend-listener -o jsonpath="{.data.service_endpoint}" | base64 --decode
http://backend-listener:3000

❯ kubectl get secret backend-listener -o jsonpath="{.data.route_endpoint}" | base64 --decode
https://backend-3scale.example.net

Upgrade from 2.13

Create new project

oc new-project 3scale-testing-upgrade

Checkout current master branch and run the operator

git checkout 8cc98759cc1ee761bf5fb811fb2dcfccf122b71a
make install
make run

Deploy 3scale from current master
- no need to specify S3 credentials
- wildcardDomain must be something valid (only if it is going to be used later). Usually ${PROJECT_NAME}.apps.${CLUSTER_DOMAIN}

k apply -f - <<EOF
apiVersion: v1
kind: Secret
metadata:
  creationTimestamp: null
  name: aws-auth
stringData:
  AWS_ACCESS_KEY_ID: testID
  AWS_SECRET_ACCESS_KEY: testkey
  AWS_BUCKET: testbucket
  AWS_REGION: us-east-1
type: Opaque
EOF

k apply -f - <<EOF
---
apiVersion: apps.3scale.net/v1alpha1
kind: APIManager
metadata:
  name: apimanager1
spec:
  wildcardDomain: test01.example.net
  resourceRequirementsEnabled: false
  system:
    fileStorage:
      simpleStorageService:
        configurationSecretRef:
          name: aws-auth
EOF

wait for deployment to be ready

oc wait --for=condition=available apimanager/apimanager1 --timeout=-1s
apimanager.apps.3scale.net/apimanager1 condition met

The above "wait" ensures the pods are up and running.

Check the system-app deployment has the APICAST_BACKEND_ROOT_ENDPOINT and BACKEND_ROUTE env var. Note that BACKEND_ROUTE is set to http://backend-listener:3000/internal/

❯ k get dc system-app -o yaml | grep -A 4  APICAST_BACKEND_ROOT_ENDPOINT
          - name: APICAST_BACKEND_ROOT_ENDPOINT
            valueFrom:
              secretKeyRef:
                key: route_endpoint
                name: backend-listener
--
        - name: APICAST_BACKEND_ROOT_ENDPOINT
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener
--
        - name: APICAST_BACKEND_ROOT_ENDPOINT
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener
--
        - name: APICAST_BACKEND_ROOT_ENDPOINT
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener

❯ k get dc system-app -o yaml | grep -A 1 BACKEND_ROUTE
          - name: BACKEND_ROUTE
            value: http://backend-listener:3000/internal/
--
        - name: BACKEND_ROUTE
          value: http://backend-listener:3000/internal/
--
        - name: BACKEND_ROUTE
          value: http://backend-listener:3000/internal/
--
        - name: BACKEND_ROUTE
          value: http://backend-listener:3000/internal/

Let's run upgrade. Checkout this PR's branch and run the operator

git checkout <new-branch-name>
make run

The upgrade should happen automatically

wait for deployment to be ready

oc wait --for=condition=available apimanager/apimanager1 --timeout=-1s
apimanager.apps.3scale.net/apimanager1 condition met

The above "wait" ensures the pods are up and running.

All the deployments should be ready. Note the system-app deployment as it should have been rolled out automatically.

❯ k get apimanager apimanager1 -o jsonpath='{.status.deployments}' | yq e -P
ready:
  - apicast-production
  - apicast-staging
  - backend-cron
  - backend-listener
  - backend-redis
  - backend-worker
  - system-app
  - system-memcache
  - system-mysql
  - system-redis
  - system-searchd
  - system-sidekiq
  - zync
  - zync-database
  - zync-que

Check the system-app deployment no longer has APICAST_BACKEND_ROOT_ENDPOINT or BACKEND_ROUTE env vars

# It should report empty
❯ k get dc system-app -o yaml | grep -A 1  APICAST_BACKEND_ROOT_ENDPOINT

# It should report empty
❯ k get dc system-app -o yaml | grep -A 1 BACKEND_ROUTE

Check the system-app deployment has the BACKEND_URL env var from the backend-listener secret and service_endpoint field

❯ k get dc system-app -o yaml | grep -A 4 BACKEND_URL
          - name: BACKEND_URL
            valueFrom:
              secretKeyRef:
                key: service_endpoint
                name: backend-listener
--
        - name: BACKEND_URL
          valueFrom:
            secretKeyRef:
              key: service_endpoint
              name: backend-listener
--
        - name: BACKEND_URL
          valueFrom:
            secretKeyRef:
              key: service_endpoint
              name: backend-listener
--
        - name: BACKEND_URL
          valueFrom:
            secretKeyRef:
              key: service_endpoint
              name: backend-listener

It is replicated 4 times in 4 different deployment: pre-hook pod deployment container and system-master, system-provider, system-developer containers.

Check the system-app deployment has the BACKEND_PUBLIC_URL env var from the backend-listener secret and route_endpoint field

❯ k get dc system-app -o yaml | grep -A 4 BACKEND_PUBLIC_URL
          - name: BACKEND_PUBLIC_URL
            valueFrom:
              secretKeyRef:
                key: route_endpoint
                name: backend-listener
--
        - name: BACKEND_PUBLIC_URL
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener
--
        - name: BACKEND_PUBLIC_URL
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener
--
        - name: BACKEND_PUBLIC_URL
          valueFrom:
            secretKeyRef:
              key: route_endpoint
              name: backend-listener

The backend-listener secret values

❯ kubectl get secret backend-listener -o jsonpath="{.data.service_endpoint}" | base64 --decode
http://backend-listener:3000

❯ kubectl get secret backend-listener -o jsonpath="{.data.route_endpoint}" | base64 --decode
https://backend-3scale.example.net

openshift-ci · 2023-04-24T11:25:34Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

mayorova · 2023-04-24T13:49:53Z

pkg/3scale/amp/operator/system_options_provider.go

-	}
-
-	s.options.BackendServiceEndpoint = urlObj.String()
+	urlObj.Path += "/internal/"


This is not checking for possible trailing slashes in the base URL, so may potentially result in double slashes (which is not very critical, IMO).

In Go 1.19 the following can be done instead:

s.options.CoreInternalAPIEndpoint = urlObj.JoinPath("/internal/").String()

We are not yet there (Go 1.19). We recently updated to 1.18 #814

If we needed to upgrade the secret content (which I think it is not needed), I would remove this logic in the 3scale operator of adding internal and rely on the value of the secret. I do not know the reason why the operator needs to add this /internal to the path.

However, we do not need to upgrade the content of the secret, so I would just leave as it is:

if urlObj.Path == "" { urlObj.Path = "/internal/" } s.options.CoreInternalAPIEndpoint = urlObj.String()

For me the initial intention was to allow any custom path if required. Only if the custom path is not there, the operator adds the /internal. With urlObj.Path += "/internal/" you are breaking that behavior.

Please see my comment here that provides more contexts.

With regards to how to append the /internal/ path (only applicable if we go with the option 2), I think the option I suggested is more flexible. It permits the service URL for backend listener to have some path already.

So, if we set https://my-apisonator-stub.somedomain.com/whatever as service_endpoint (assuming apisonator is listening at that address), then the core API endpoint will be set to https://my-apisonator-stub.somedomain.com/whatever/internal correctly.

While, in the option you suggest it will not work (the /internal will not be appended).

mayorova · 2023-04-24T13:50:54Z

pkg/3scale/amp/operator/system_options_provider.go

-
-	s.options.BackendServiceEndpoint = urlObj.String()
+	urlObj.Path += "/internal/"
+	s.options.CoreInternalAPIEndpoint = urlObj.String()


Currently the PR builds the CoreInternalAPIEndpoint here to set its value to THREESCALE_CORE_INTERNAL_API env var, but I am wondering if it could make sense to remove this completely, and instead have the operator only set the env vars for the route (BACKEND_PUBLIC_URL) and the internal service (e.g. call it BACKEND_URL), and append /internal/ path in porta configuration (e.g. "#{BACKEND_URL}/internal/")

Be my guest

With the current implementation in this PR, there are three env vars

APICAST_BACKEND_ROOT_ENDPOINT -> https://backend-3scale.eguzki-3scale.apps.example.com

THREESCALE_CORE_INTERNAL_API -> http://backend-listener:3000/internal

BACKEND_PUBLIC_URL -> https://backend-3scale.eguzki-3scale.apps.example.com

APICAST_BACKEND_ROOT_ENDPOINT and BACKEND_PUBLIC_URL will always have the same value. For now at least. Seems duplicated, shall we simplify it?

For me it seems like we need two of them:

BACKEND_PUBLIC_URL -> https://backend-3scale.eguzki-3scale.apps.example.com

BACKEND_INTERNAL_URL -> http://backend-listener:3000/internal

In system we still need the backend internal URL without the /internal prefix, more info on that in this comment.

As for APICAST_BACKEND_ROOT_ENDPOINT and BACKEND_PUBLIC_URL indeed the values are duplicated, and I also thought about whether we need to merge them into one.

I think there could potentially be a scenario where the customer would want to have different values.

BACKEND_PUBLIC_URL would be used by Active Docs (so that the customer can make requests to the Service Management API from the browser)

APICAST_BACKEND_ROOT_ENDPOINT is used by APIcast (so that APIcast can make requests to the Service Management API)

So, depending on where APIcast is deployed (if it's outside of OCP), maybe these two values could be different (within private network vs public).

On the other hand, we could indeed make it simple, and just use one value, and if somebody needs to distinguish these two, they can open an RFE.

For now, I maintained both APICAST_BACKEND_ROOT_ENDPOINT and BACKEND_PUBLIC_URL, even though they have the same value.

eguzki · 2023-04-28T13:07:33Z

Missing upgrade procedure. Let's say 3scale is running 2.13. The new 2.14 is deployed. The operator will needs to do the changes, removing deprecated env vars and adding new ones to the existing deployment config.

I can help with that procedure.

mayorova · 2023-05-02T09:47:04Z

I see. Thanks @eguzki!

Do you maybe have an example of a similar upgrade?

eguzki · 2023-05-02T11:32:57Z

example

https://github.com/3scale/3scale-operator/blob/ed88dd9e4543da43a8554ab95cab56334593a9c0/pkg/upgrade/upgrade_sphinx_reference_2.14.go

First, I would decide on what we do and then execute

mayorova · 2023-05-10T11:50:52Z

Thank you for the comments @eguzki !

Let me provide some context (which I should have done upon opening the PR).

The backend-listener secret has two values:

route_endpoint, e.g. https://backend-3scale.apps.mycluster.com
service_endpoint: http://backend-listener:3000

From system component, for different things we need different values.

Backend (apisonator) also exposes different APIs, one is the "transactions" one, and the other one is called Core API, which is behind /internal prefix, this API is used to push the changes from system to backend (one-way sync).

I am just trying to figure out what's the best way to append this /internal prefix.

We can do it in multiple places:

In the backend-listener secret (in addition to the existing route_endpoint and service_endpoint we could add e.g. core_api_endpoint, that will have the value http://backend-listener:3000/internal)

This is the most flexible, because it would allow the user to configure the route themselves. On the other hand - why would they do that? The only use case I can think of is to be able to mock this API, but in this case the whole apisonator should be stubbed/mocked, I think.
In real life it has 0 sense to point somewhere else only for the internal API, because then system and backend will get out of sync.

In the operator, which adds the /internal prefix to the value of the service_endpoint field of the backend-listener secret, the value is then placed in the THREESCALE_CORE_INTERNAL_API env var, which is read by porta
In the porta config files.
In this case the operator would only set environment variables:

BACKEND_URL - service_endpoint value from backend-listener secret (i.e. http://backend-listener:3000)
BACKEND_PUBLIC_URL - route_endpoint from backend-listener secret (i.e. https://backend-3scale.apps.mycluster.com)

Then porta configuration file would need to append /internal prefix.

@3scale/system what do you think?

I am more inclined to option 3 to be honest. I think the operator doesn't need to know the internals - just "connect" system to backend, and let them figure out their communication on their own.

mayorova · 2023-05-10T12:09:54Z

While writing my previous comment, I actually realized that we are currently missing this environment variable:

BACKEND_URL - service_endpoint value from backend-listener secret (i.e. http://backend-listener:3000)

And this is because this value is hardcoded in the porta config: https://github.com/3scale/porta/blob/3scale-2.13.2-GA/openshift/system/config/backend.yml#L2

I think it's not cool, and we need to add this env var.

Latest master is prepared for this env var: https://github.com/3scale/porta/blob/3996062/openshift/system/config/backend.yml#L2

akostadinov · 2023-05-10T12:16:05Z

Agreed that porta and backend better figure out between themselves. Also why provide two values that are basically the same except for the /internal suffix?

mayorova · 2023-05-10T12:53:23Z

Thanks @akostadinov So, you agree with the 3rd option?

mayorova · 2023-05-15T15:21:02Z

I'm not sure why assets-validate check fails, the file it references is auto-generated.

eguzki · 2023-05-19T12:38:33Z

I'm not sure why assets-validate check fails, the file it references is auto-generated.

Because of this https://github.com/3scale/3scale-operator/pull/821/files#diff-559feb95c027cff90747b7e89b79d2886afcc3b13c7741a25752981b64236f20L292

Contribution to 821

codeclimate · 2023-05-23T12:57:48Z

Code Climate has analyzed commit 663071b and detected 0 issues on this pull request.

View more on Code Climate.

openshift-ci bot added the do-not-merge/work-in-progress label Apr 24, 2023

mayorova force-pushed the improve-backend-url-naming branch from 1fe7039 to 393dc68 Compare April 24, 2023 11:26

mayorova mentioned this pull request Apr 24, 2023

THREESCALE-9558: Rename env vars for backend URLs 3scale/porta#3324

Merged

mayorova marked this pull request as ready for review April 24, 2023 13:48

mayorova requested a review from a team as a code owner April 24, 2023 13:48

openshift-ci bot removed the do-not-merge/work-in-progress label Apr 24, 2023

mayorova commented Apr 24, 2023

View reviewed changes

mayorova force-pushed the improve-backend-url-naming branch from 0684f0e to 90cb5e2 Compare April 24, 2023 14:06

mayorova mentioned this pull request May 15, 2023

Change env var from BACKEND_ROUTE to BACKEND_URL 3scale-ops/saas-operator#260

Merged

openshift-merge-robot added the needs-rebase label May 15, 2023

mayorova added 3 commits May 15, 2023 16:21

Separate env vars for Core API and public route

a130b7c

Make compatible with Go 1.17

565d161

Use BACKEND_URL and BACKEND_PUBLIC_URL env vars in system DC

68c5875

mayorova force-pushed the improve-backend-url-naming branch from 0639d2d to 68c5875 Compare May 15, 2023 14:23

openshift-merge-robot removed the needs-rebase label May 15, 2023

Fix comment

3caf85b

eguzki and others added 3 commits May 19, 2023 15:14

fix bindata

f4b9d7a

apply SystemBackendUrls upgrade

7950785

Merge pull request #1 from eguzki/eguzki-contribution-to-821

663071b

Contribution to 821

eguzki approved these changes May 23, 2023

View reviewed changes

eguzki merged commit 9a09a3c into 3scale:master May 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

THREESCALE-9558: Separate env vars for Core API and public route #821

THREESCALE-9558: Separate env vars for Core API and public route #821

mayorova commented Apr 24, 2023 •

edited by eguzki

Loading

openshift-ci bot commented Apr 24, 2023

mayorova Apr 24, 2023

eguzki Apr 28, 2023

mayorova May 10, 2023

mayorova Apr 24, 2023

eguzki Apr 28, 2023 •

edited

Loading

mayorova May 10, 2023

mayorova May 15, 2023

eguzki commented Apr 28, 2023

mayorova commented May 2, 2023

eguzki commented May 2, 2023

mayorova commented May 10, 2023

mayorova commented May 10, 2023

akostadinov commented May 10, 2023

mayorova commented May 10, 2023

mayorova commented May 15, 2023

eguzki commented May 19, 2023

codeclimate bot commented May 23, 2023

THREESCALE-9558: Separate env vars for Core API and public route #821

THREESCALE-9558: Separate env vars for Core API and public route #821

Conversation

mayorova commented Apr 24, 2023 • edited by eguzki Loading

What

Verification steps

New deployment

Upgrade from 2.13

openshift-ci bot commented Apr 24, 2023

mayorova Apr 24, 2023

Choose a reason for hiding this comment

eguzki Apr 28, 2023

Choose a reason for hiding this comment

mayorova May 10, 2023

Choose a reason for hiding this comment

mayorova Apr 24, 2023

Choose a reason for hiding this comment

eguzki Apr 28, 2023 • edited Loading

Choose a reason for hiding this comment

mayorova May 10, 2023

Choose a reason for hiding this comment

mayorova May 15, 2023

Choose a reason for hiding this comment

eguzki commented Apr 28, 2023

mayorova commented May 2, 2023

eguzki commented May 2, 2023

mayorova commented May 10, 2023

mayorova commented May 10, 2023

akostadinov commented May 10, 2023

mayorova commented May 10, 2023

mayorova commented May 15, 2023

eguzki commented May 19, 2023

codeclimate bot commented May 23, 2023

mayorova commented Apr 24, 2023 •

edited by eguzki

Loading

eguzki Apr 28, 2023 •

edited

Loading