8000 THREESCALE-10162 add skip insecure verify to OpenAPI backend and product by MStokluska · Pull Request #869 · 3scale/3scale-operator · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

THREESCALE-10162 add skip insecure verify to OpenAPI backend and product #869

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 27, 2023
Merged

THREESCALE-10162 add skip insecure verify to OpenAPI backend and product #869

merged 1 commit into from
Sep 27, 2023

Conversation

MStokluska
Copy link
Contributor
@MStokluska MStokluska commented Sep 20, 2023 •
edited
Loading

Jira: https://issues.redhat.com/browse/THREESCALE-10162

Verification

Verification environment preparation

  • Create new project
oc new-project threescale
  • Install CRDs
make install
  • Run the operator
make run
  • Create new secret to be consumed by APIM
oc apply -f - <<EOF   
---
apiVersion: v1
kind: Secret
metadata:
  creationTimestamp: null
  name: s3-credentials
stringData:
  AWS_ACCESS_KEY_ID: something
  AWS_SECRET_ACCESS_KEY: something
  AWS_BUCKET: something
  AWS_REGION: us-east-1
type: Opaque
EOF
  • Fetch cluster domain
DOMAIN=$(oc get routes console -n openshift-console -o json | jq -r '.status.ingress[0].routerCanonicalHostname' | sed 's/router-default.//')
  • Create APIM
kubectl apply -f - <<EOF                                                        
---
apiVersion: apps.3scale.net/v1alpha1
kind: APIManager
metadata:
  name: apimanager-sample
  namespace: threescale
spec:
  system:
    fileStorage:
      simpleStorageService:
        configurationSecretRef:
          name: s3-credentials
  wildcardDomain: $DOMAIN
EOF
  • Confirm 3scale is fully installed
oc get apimanager apimanager-sample -o json | jq -r '.status.deployments'
  • Create OpenAPI secret
kubectl apply -f - <<EOF               
---
kind: Secret
apiVersion: v1
metadata:
  name: myopenapi
stringData:
 myopenapi.yaml: |
  ---
  openapi: "3.0.2"
  info:
    title: "testinsecure"
    description: "some description"
    version: "1.0.0"
  servers:
   - url: https://echo-api.3scale.net:443
  paths:
    /:
      get:
        operationId: "get"
        responses:
          405:
            description: "invalid input"
type: Opaque
EOF
  • Create OpenAPI CR:
kubectl apply -f - <<EOF                                                        
---
apiVersion: capabilities.3scale.net/v1beta1
kind: OpenAPI
metadata:
  name: ownertest
  namespace: threescale
  annotations:
     "insecure_skip_verify": "true"
spec:
  openapiRef:
    secretRef:
      name: myopenapi
      namespace: threescale
  productSystemName: testProduct
EOF
  • Confirm OpenAPI resources have been created (product and backend) and that both resources have the insecure_skip_verify set to "true" and are created in 3scale
  • Delete OpenAPI CR and re-create it, this time with insecure_skip_verify set to false, confirm both resources have the flag set to false and that both existing in 3scale

@MStokluska MStokluska requested a review from a team as a code owner September 20, 2023 07:58
@eguzki
Copy link
Member
eguzki commented Sep 20, 2023

What is the use case for having backend_insecure_skip_verify and product_insecure_skip_verify two annotations?

@MStokluska
Copy link
Contributor Author

What is the use case for having backend_insecure_skip_verify and product_insecure_skip_verify two annotations?

@eguzki do we want to give individual control over them - reading your comment as I guess you are suggesting having just one right?

@MStokluska
Copy link
Contributor Author

What is the use case for having backend_insecure_skip_verify and product_insecure_skip_verify two annotations?

@eguzki do we want to give individual control over them - reading your comment as I guess you are suggesting having just one right?

It actually makes no point in having two, made it a single annotation instead. Thanks

@MStokluska MStokluska force-pushed the THREESCALE-10162 branch 2 times, most recently from eedac1f to fadef92 Compare September 21, 2023 08:35
@openshift-ci
Copy link
openshift-ci bot commented Sep 21, 2023

@MStokluska: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test test-e2e
  • /test test-unit

Use /test all to run all jobs.

In response to this:

/retest ci/circleci: doc-toc-validate

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@codeclimate
Copy link
codeclimate bot commented Sep 21, 2023

Code Climate has analyzed commit 5b7aa73 and detected 0 issues on this pull request.

View more on Code Climate.

@carlkyrillos
Copy link
Contributor

Not sure why the ci/circleci: doc-toc-validate test is failing but I reviewed the changed files and verified that that the "insecure_skip_verify": "true|false" annotation is correctly propagated from the OpenAPI CR to the Product and Backend CRs.
/lgtm

@MStokluska
Copy link
Contributor Author

@eguzki we are about to merge this. Given you have left some feedback - are you happy with the current approach or think there is more changes required?

eguzki
eguzki reviewed Sep 25, 2023
View reviewed changes
Copy link
Member
@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

LGTM

Three notes:

  • The product/backend annotations are not reconciled, so if, for instance, not added initially to the OpenAPI CR and then update the CR to add it, the product/backend CR should not get that annotation.
  • The annotation is always added to the product/backend CR, regardless of existing in or not in the OpenAPI CR. I would not add annotation if OpenAPI does not have it. Minor issue. Up to you.
  • Doc test failing

@MStokluska
Copy link
Contributor Author

LGTM

Three notes:

* The product/backend annotations are not reconciled, so if, for instance, not added initially to the OpenAPI CR and then update the CR to add it, the product/backend CR should not get that annotation.

* The annotation is always added to the product/backend CR, regardless of existing in or not in the OpenAPI CR. I would not add annotation if OpenAPI does not have it. Minor issue. Up to you.

* Doc test failing

Thanks @eguzki
Unfortunately we weren't able to figure out what's up with toc make commands but it's affected on master as well. Until we resolve it all PRs will be failing on toc check.

austincunningham
austincunningham approved these changes Sep 25, 2023
View reviewed changes
Copy link
Contributor
@austincunningham austincunningham left a comment

Choose a reason for hiding this comment

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

/lgtm

@eguzki
Copy link
Member
eguzki commented Sep 25, 2023

Indeed, in master gh-md-toc doc/operator-user-guide.md is returning empty (NO TOC), maybe doc/operator-user-guide.md is somewhat corrupted?

@MStokluska MStokluska merged commit 6a9f5c7 into 3scale:master Sep 27, 2023
@briangallagher briangallagher mentioned this pull request Jan 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eguzki eguzki eguzki left review comments

@austincunningham austincunningham austincunningham approved these changes

Assignees
No one assigned
Labels
approve lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MStokluska @eguzki @carlkyrillos @austincunningham
0