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

Enhancement: added --insecure-tls #926

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

eunames
Copy link

@eunames eunames commented Feb 21, 2024

Summary

  • Short summary of what's included in the PR
  • Give special note to breaking changes: List the exact changes or provide links to documentation.

Checklist

For Code changes

  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • PR contains the label area:operator
  • Link this PR to related issues
  • I have not made any changes in the charts/ directory.

Added the oportunity to set a flag --insecure-tls for the restic command.
If you want to set the flag --insecure-tls you should set env SET_INSECURE_TLS_FLAG to true in deployment of k8up operator.

Example:

  template:
    spec:
      containers:
      - args:
        - operator
        env:
        - name: BACKUP_ENABLE_LEADER_ELECTION
          value: "true"
        - name: BACKUP_OPERATOR_NAMESPACE
          valueFrom:
            fieldRef:
              apiVersion: v1
              fieldPath: metadata.namespace
        - name: SET_INSECURE_TLS_FLAG
          value: "true"

fixed:
#792
#882
#881

Signed-off-by: eunames <[email protected]>
@eunames eunames requested a review from a team as a code owner February 21, 2024 08:34
@eunames eunames requested review from zugao and lieneluksika and removed request for a team February 21, 2024 08:34
Copy link
Contributor

@Kidswiss Kidswiss left a comment

Choose a reason for hiding this comment

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

Hi @eunames

Thank you for the PR!

I feel like adding this as an operator level configuration is not very flexible. With this each and every backup either has it enabled or disabled.

This might be fine for a k8s cluster with only one tenant, however K8up is frequently used in a multi-tenant environment. Setting a security critical configuration globally is not a good idea in such a scenario, as it could compromise the security of the tenants.

So instead, it would be the more flexible approach to add a new flag in the backup CRD and then set SET_INSECURE_TLS_FLAG accordingly on the restic jobs. This way each tenant can configure the setting for each backup individually.

Can you please have a look at my suggestion? From what you already have it shouldn't be too hard to add it to the CRD and pass it to the restic jobs.

@eunames
Copy link
Author

eunames commented Apr 1, 2024

Hi
It a more flexible solution i think.
Now customer can set SET_INSECURE_TLS_FLAG in an operator level like was before and/or in a backup card.
So now customer can set flag in the deployment of k8up operator

 template:
     spec:
       containers:
       - args:
         - operator
         env:
         - name: BACKUP_ENABLE_LEADER_ELECTION
           value: "true"
         - name: BACKUP_OPERATOR_NAMESPACE
           valueFrom:
             fieldRef:
               apiVersion: v1
               fieldPath: metadata.namespace
         - name: SET_INSECURE_TLS_FLAG
           value: "true"

Or/and

in the backup card set a flag insecureTLS

apiVersion: k8up.io/v1
kind: Backup
metadata:
  name: backup-true
spec:
  failedJobsHistoryLimit: 2
  successfulJobsHistoryLimit: 2
  backend:
    repoPasswordSecretRef:
    	.....
    insecureTLS: true
    s3:
    	....

A value TRUE in the operator level has priority over a value FALSE in the backup card

But I'm not sure that I've considered all the options when customer can use an insecire connection!!! and testing...... Sorry :(

@Kidswiss
Copy link
Contributor

Kidswiss commented Apr 5, 2024

A value TRUE in the operator level has priority over a value FALSE in the backup card

This feels somewhat like the wrong way around. All other operator level options have less precedence than the options in the backup object.

Also, are you still testing, or is this ready for a re-review from my side?

@eunames
Copy link
Author

eunames commented Apr 5, 2024

Ready.
Unfortunate, i can't start buil-in tests (kind's finished with error) and do manual testing at a good level. I think a probability of errors is high.

@Kidswiss
Copy link
Contributor

Ready. Unfortunate, i can't start buil-in tests (kind's finished with error) and do manual testing at a good level. I think a probability of errors is high.

You're right, a few tests are currently failing. Could you take a look at them?

commit 1e7871c
Merge: d996d16 0f1228f
Author: Bigli <[email protected]>
Date:   Tue May 28 13:17:19 2024 +0200

    Merge pull request k8up-io#974 from k8up-io/gh/pr_template_co_sign

    Adjust the PR template to include signing off the commits

commit 0f1228f
Author: Nicolas Bigler <[email protected]>
Date:   Tue May 28 12:44:20 2024 +0200

    Adjust the PR template to include signing off the commits

    Signed-off-by: Nicolas Bigler <[email protected]>

commit d996d16
Merge: 0b29883 c0efc71
Author: Bigli <[email protected]>
Date:   Tue May 28 12:41:08 2024 +0200

    Merge pull request k8up-io#967 from SchoolGuy/add-grafana-dashboard

    feat: Helm - Grafana Dashboard

commit c0efc71
Author: Enno Gotthold <[email protected]>
Date:   Thu May 9 12:35:41 2024 +0200

    [ADD] Helm - Grafana Dashboard

    Signed-off-by: Enno Gotthold <[email protected]>
    Signed-off-by: Nicolas Bigler <[email protected]>

commit 0b29883
Merge: 60e75b6 f6648dd
Author: Bigli <[email protected]>
Date:   Tue May 28 10:18:14 2024 +0200

    Merge pull request k8up-io#971 from k8up-io/tutorial/update_crd_version

    Update the tutorial to the latest k8up version

commit f6648dd
Author: Nicolas Bigler <[email protected]>
Date:   Mon May 27 15:31:27 2024 +0200

    Update the tutorial to the latest k8up version

    Signed-off-by: Nicolas Bigler <[email protected]>

commit 60e75b6
Merge: 212a033 162e16b
Author: Kidswiss <[email protected]>
Date:   Wed May 15 10:20:29 2024 +0200

    Merge pull request k8up-io#969 from k8up-io/bump_chart

    Bump K8up version

commit 162e16b
Author: Simon Beck <[email protected]>
Date:   Wed May 15 09:45:16 2024 +0200

    Bump K8up version

    Signed-off-by: Simon Beck <[email protected]>

commit 212a033
Merge: 2ddb6ee c8e9202
Author: Kidswiss <[email protected]>
Date:   Wed May 15 09:36:03 2024 +0200

    Merge pull request k8up-io#968 from k8up-io/add/pod_spec

    Add full podSpec to all job types

commit c8e9202
Author: Simon Beck <[email protected]>
Date:   Wed May 15 09:04:58 2024 +0200

    Remove unnecessary RBAC

    Signed-off-by: Simon Beck <[email protected]>

commit d21f019
Author: Simon Beck <[email protected]>
Date:   Tue May 14 16:01:26 2024 +0200

    Correctly flag test for integration

    Signed-off-by: Simon Beck <[email protected]>

commit edea9f6
Author: Simon Beck <[email protected]>
Date:   Tue May 14 15:29:02 2024 +0200

    Add make test to actions

    Signed-off-by: Simon Beck <[email protected]>

commit 21494ca
Author: Simon Beck <[email protected]>
Date:   Mon May 13 13:26:56 2024 +0200

    Add docs

    Signed-off-by: Simon Beck <[email protected]>

commit 2666a12
Author: Simon Beck <[email protected]>
Date:   Mon May 13 12:50:07 2024 +0200

    Fix using the index to delay execution

    It did not actually do a staggered delay for each instance.

    Signed-off-by: Simon Beck <[email protected]>

commit dd9f657
Author: Simon Beck <[email protected]>
Date:   Mon May 13 12:49:52 2024 +0200

    Add full podSpec to all job types

    With this commit it's now possible to specify a full podSpec for each
    job type available.

    Signed-off-by: Simon Beck <[email protected]>

commit 2ddb6ee
Merge: 5ab0be4 15f78ac
Author: Kidswiss <[email protected]>
Date:   Tue Apr 30 15:34:54 2024 +0200

    Merge pull request k8up-io#963 from k8up-io/bump-chart

    Bump versions in the chart

commit 15f78ac
Author: Simon Beck <[email protected]>
Date:   Tue Apr 30 15:10:27 2024 +0200

    Bump versions in the chart

    Signed-off-by: Simon Beck <[email protected]>

commit 5ab0be4
Merge: 5d67b16 51b7fd7
Author: Kidswiss <[email protected]>
Date:   Tue Apr 30 14:47:07 2024 +0200

    Merge pull request k8up-io#962 from amghazanfari/master

    bug: Add operator as mandatory argument

commit 51b7fd7
Author: Amir M. Ghazanfari <[email protected]>
Date:   Sun Apr 28 12:37:15 2024 +0330

    Add operator as mandatory argument

    Signed-off-by:  amghazanfari <[email protected]>
    Signed-off-by: Amir Ghazanfari <[email protected]>

commit 5d67b16
Merge: 2971c49 ccd6bce
Author: Kidswiss <[email protected]>
Date:   Fri Apr 12 11:12:03 2024 +0200

    Merge pull request k8up-io#954 from M0NsTeRRR/master

    [enhancement] add support for dual stack clusters

commit 2971c49
Merge: 1088118 80b2ddd
Author: Kidswiss <[email protected]>
Date:   Fri Apr 12 10:22:25 2024 +0200

    Merge pull request k8up-io#949 from poyaz/feature/custom-tls

    [enhancement] Adding new feature for supporting self-signed certificate

commit 80b2ddd
Author: poyaz <[email protected]>
Date:   Thu Apr 11 17:33:31 2024 +0330

    [ADD] Add integration test for TLS and Mutual TLS options

    Signed-off-by: poyaz <[email protected]>

commit ad78959
Author: poyaz <[email protected]>
Date:   Thu Apr 11 17:32:40 2024 +0330

    [FIX] Fix execute ps for alpine and BusyBox

    Signed-off-by: poyaz <[email protected]>

commit 1fc3b16
Author: poyaz <[email protected]>
Date:   Thu Apr 11 17:32:05 2024 +0330

    [UPDATE] Rename argument "--varDir" to "-varDir"

    Signed-off-by: poyaz <[email protected]>

commit df889cb
Author: poyaz <[email protected]>
Date:   Thu Apr 11 17:14:02 2024 +0330

    [UPDATE] Add unit test for utils file and refactoring ZeroLen function

    Signed-off-by: poyaz <[email protected]>

commit 22de53e
Author: poyaz <[email protected]>
Date:   Thu Apr 11 00:56:30 2024 +0330

    [DELETE] Delete e2e test self signed tls becuase it has too many test case and spend too much time

    Move restore and archive test case to two separated files

    Signed-off-by: poyaz <[email protected]>

commit 7d121ff
Author: poyaz <[email protected]>
Date:   Thu Apr 11 00:53:42 2024 +0330

    [ADD] Add two e2e test for restore and archive

    Signed-off-by: poyaz <[email protected]>

commit dc9f803
Author: poyaz <[email protected]>
Date:   Thu Apr 11 00:52:56 2024 +0330

    [ADD] Add cmctl command for check cert-manager is ready

    Signed-off-by: poyaz <[email protected]>

commit 0acef98
Author: poyaz <[email protected]>
Date:   Thu Apr 11 00:32:34 2024 +0330

    [UPDATE] Refactoring code for duplciate fucntions in operators

    These functions is created in utils:
    - AppendTLSOptionsArgs: for generate env for backend and restore specs
    - AttachTLSVolumes: for create volumes for pods
    AttachTLSVolumeMounts: for create volumeMount for backend and restore specs

    Signed-off-by: poyaz <[email protected]>

commit b59589a
Author: poyaz <[email protected]>
Date:   Thu Apr 11 00:30:11 2024 +0330

    [UPDATE] Update documents because of changing options to tlsOptions

    Signed-off-by: poyaz <[email protected]>

commit 01cb120
Author: Pooya Azarpour <[email protected]>
Date:   Mon Apr 8 14:21:24 2024 +0330

    [CHANGE] Rename options to tlsOptions

    Signed-off-by: Pooya Azarpour <[email protected]>

commit 9b4216a
Author: Pooya Azarpour <[email protected]>
Date:   Mon Apr 8 11:01:50 2024 +0330

    [DELETE] Delete unnecessary error param in setupArgs function

    Signed-off-by: Pooya Azarpour <[email protected]>

commit ccd6bce
Author: Ludovic Ortega <[email protected]>
Date:   Sat Apr 6 17:59:17 2024 +0200

    feat: add support for dual stack clusters

    Signed-off-by: Ludovic Ortega <[email protected]>

commit d1319f0
Author: Pooya Azarpour <[email protected]>
Date:   Sat Apr 6 13:43:25 2024 +0330

    [FIX] Fix typo and document's grammers

    Signed-off-by: Pooya Azarpour <[email protected]>

commit 8713c81
Author: Pooya Azarpour <[email protected]>
Date:   Sat Apr 6 13:40:33 2024 +0330

    [UPDATE] Formatting go files to old style (Remove idea customziation formatter)

    Signed-off-by: Pooya Azarpour <[email protected]>

commit f6b0f12
Author: Pooya Azarpour <[email protected]>
Date:   Sat Apr 6 11:20:33 2024 +0330

    [DELETE] Delete command "sleep 3"

    Signed-off-by: Pooya Azarpour <[email protected]>

commit 1088118
Merge: b16b4bf 29cdb0a
Author: Tobias Brunner <[email protected]>
Date:   Thu Apr 4 08:36:45 2024 +0200

    Merge pull request k8up-io#952 from halil-bugol/patch-1

    Add Kubezy as adopter

commit 29cdb0a
Author: Halil İbrahim BUGÖL <[email protected]>
Date:   Thu Apr 4 00:53:48 2024 +0300

    Update ADOPTERS.md

    Signed-off-by: Halil Bugol <[email protected]>
    Signed-off-by: Halil İbrahim BUGÖL <[email protected]>

commit 9f776fe
Author: poyaz <[email protected]>
Date:   Sat Mar 23 17:05:19 2024 +0330

    [FIX] Fix test for expected args

    Signed-off-by: poyaz <[email protected]>

commit 295e5bf
Author: poyaz <[email protected]>
Date:   Sat Mar 23 16:55:12 2024 +0330

    [ADD] Adding variable GO_EXEC in Makefile to choose different versions of Golang

    Signed-off-by: poyaz <[email protected]>

commit c668b25
Author: poyaz <[email protected]>
Date:   Sat Mar 23 16:53:54 2024 +0330

    [FIX] Fixing integration test for restic s3. Missing CaCert arguments

    Signed-off-by: poyaz <[email protected]>

commit da60a0b
Author: poyaz <[email protected]>
Date:   Sat Mar 23 15:03:13 2024 +0330

    ADD] Adding e2e test over using env for TLS and mTls

    Also fixing bug in get lentgh of archive object in minio-mc

    Signed-off-by: poyaz <[email protected]>

commit 41825f9
Author: poyaz <[email protected]>
Date:   Sat Mar 23 15:03:06 2024 +0330

    [ADD] Adding e2e definitaions for using env for TLS and mTls

    Signed-off-by: poyaz <[email protected]>

commit bd2c880
Author: poyaz <[email protected]>
Date:   Sat Mar 23 15:01:04 2024 +0330

    [UPDATE] Update cert-manager to v1.14.4

    Signed-off-by: poyaz <[email protected]>

commit 6b659b8
Author: poyaz <[email protected]>
Date:   Sat Mar 23 13:14:10 2024 +0330

    [UPDATE] Update api-refrence according to supporting volume, volumeMount, and options

    Signed-off-by: poyaz <[email protected]>

commit b2b83e1
Author: poyaz <[email protected]>
Date:   Sat Mar 23 13:12:30 2024 +0330

    [ADD] Adding document about how to use TLS and mTls in api refrence

    Signed-off-by: poyaz <[email protected]>

commit fe51211
Author: poyaz <[email protected]>
Date:   Sat Mar 23 13:12:03 2024 +0330

    [FIX] Removing unnecessary snipped tag (tag: <SNIP>)

    Signed-off-by: poyaz <[email protected]>

commit 800b819
Author: poyaz <[email protected]>
Date:   Sat Mar 23 13:10:27 2024 +0330

    [UPDATE] Update operator and restic cli help according to new values is added

    Signed-off-by: poyaz <[email protected]>

commit 11f0945
Author: poyaz <[email protected]>
Date:   Sat Mar 23 13:09:48 2024 +0330

    [ADD] Adding RESTORE_CA_CERT_FILE, RESTORE_CA_CERT_FILE, RESTORE_CLIENT_KEY_FILE env instead of filling TLS and mTls options in restore method

    Signed-off-by: poyaz <[email protected]>

commit a9cf8fd
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:08:07 2024 +0330

    [FIX] Fixning problem in attach mode when failer happend in pod

    Signed-off-by: poyaz <[email protected]>

commit 97e03e0
Merge: 5270d54 b16b4bf
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:06:15 2024 +0330

    Merge remote-tracking branch 'upstream/master' into feature/custom-tls

    Signed-off-by: poyaz <[email protected]>

commit 5270d54
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:05:02 2024 +0330

    [ADD] Adding new e2e test for supporting self-signed issuer

    This test contains below sections:
    - Testing backup API for TLS and mTLS mode
    - Testing restore API in pvc for TLS and mTLS mode
    - Testing restore API in S3 for TLS and mTLS mode
    - Testing archive API in S3 for TLS and mTLS mode
    - Testin check API for TLS and mTLS mode

    Signed-off-by: poyaz <[email protected]>

commit f391110
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:04:44 2024 +0330

    [ADD] Add some fucntions for checking e2e test

    These fucntions add:
    - Adding "mc" function for using minio client for using download files, remove buckets, get list of files
    - Adding "given_a_clean_archive" function for clear archive bucket
    - Adding "given_a_subject_dl" function for apply deployment for checking last backup when restore in S3
    - Adding "give_self_signed_issuer" function for create self-signed issuer
    - Adding "expect_dl_file_in_container" function for checking is last backup was uploaded in S3 is okay

    Also fix some bugs:
    - Fixing empty output when get last dump of snapshot - becuase of syncing and storing file in disk, fetching last dump is took and the output of "run restic dump latest" is empty
    - Adding sleep before running restic and mc

    Signed-off-by: poyaz <[email protected]>

commit 2af7fd6
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:04:23 2024 +0330

    [ADD] Adding new resource definitions for e2e test in TLS and mTls mode

    These definitions contain below:
    - Adding archive
    Adding restore
    Adding backup
    Adding nginx for use reverse proxy in TLS and mTls mode
    Adding cert-manager for genrate self-signed issuer

    Signed-off-by: poyaz <[email protected]>

commit a56d465
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:03:58 2024 +0330

    [UPDATE] Generating new crd according to adding VolumeMounts to BackendSpec and RestoreMethodSpec

    Signed-off-by: poyaz <[email protected]>

commit ece84c7
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:03:46 2024 +0330

    [UPDATE] Addin VolumeMounts to BackendSpec and RestoreMethod

    Change:
    - Removing VolumeMounts from S3Spec and RestServerSpec in BackendSpec. Adding to BackendSpec (File: v1/backend.go)
    - Adding VolumeMounts to RestoreMethod (File: v1/restore_types.go)

    Signed-off-by: poyaz <[email protected]>

commit c42e748
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:02:47 2024 +0330

    [UPDATE] Generating new crd according to adding VolumeMounts to BackendSpec and RestoreMethodSpec

    Also these changes appends:
    - Running linter
    - Fixing check null pointer error if BackendSpec or Volume of Spec is null
    - Fixing check add duplicate VolumeMount in archive and restore API
    - Refactoring setupArgs

    Signed-off-by: poyaz <[email protected]>

commit a0f78d7
Author: poyaz <[email protected]>
Date:   Sat Mar 23 02:01:14 2024 +0330

    [ADD] Adding container volumes when they are mounting

    Signed-off-by: poyaz <[email protected]>

commit e2622c6
Author: Pooya Azarpour <[email protected]>
Date:   Mon Mar 18 19:45:22 2024 +0330

    [ADD] Supporting self certificate authority and mTls when using S3 object storage

    Signed-off-by: Pooya Azarpour <[email protected]>

commit e13ba45
Author: Pooya Azarpour <[email protected]>
Date:   Mon Mar 18 19:43:39 2024 +0330

    [ADD] Add vardir command option for mount emptyDir in pod

    Signed-off-by: Pooya Azarpour <[email protected]>

commit 8eb0703
Author: Pooya Azarpour <[email protected]>
Date:   Mon Mar 18 19:41:40 2024 +0330

    [ADD] Add Volume for using secret or configmap in k8s, Add VolumeMounts for mount volume, Add BackendOpts for using custom options in k8up or restic

    Signed-off-by: Pooya Azarpour <[email protected]>

commit 2ea688a
Author: Pooya Azarpour <[email protected]>
Date:   Mon Mar 18 19:39:05 2024 +0330

    [ADD] Add GO_EXEC variable for using multiply version of go binary

    Signed-off-by: Pooya Azarpour <[email protected]>

commit 43f750c
Author: Pooya Azarpour <[email protected]>
Date:   Mon Mar 18 19:37:59 2024 +0330

    [ADD] Ignoring vagrant dir in git

    Signed-off-by: Pooya Azarpour <[email protected]>
Signed-off-by: eunames <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants