fix(e2e): make the oracle suite runnable (#6770)

The oracle suite could not have run at any point. Two of the reasons were known: it
reads OCI_* while CI and run.sh supply ORACLE_*, and the SecretStore hardcodes the vault
as the string "vaultOCID". Fixing those exposed six more, all of the kind that only
survive in code nothing executes.

The provider struct declared a context field and never assigned it, so both API calls
passed nil and would have panicked in WithContext. CreateSecretDetails requires
CompartmentId, KeyId, SecretName and VaultId; three were never set. Every secret was
named the package constant "secretName" with the caller's key going to the content name,
so no remoteRef.key could resolve, since GetSecret looks a secret up by name, and every
spec in a run would have collided on one name. BeforeEach created a Secret holding the
literal string "value" while the store referenced a different Secret, with different
keys, that nothing created. DeleteSecret passed a name where an OCID is required. And
content was sent raw where the SDK documents base64.

Adopt the ORACLE_* names rather than move CI to OCI_*: they are what run.sh and the
workflows already supply, they match the oracle secret group, and they match every
sibling provider in the same file. Add ORACLE_VAULT_OCID, ORACLE_COMPARTMENT_OCID and
ORACLE_ENCRYPTION_KEY_OCID, which the Vault API requires to create a secret at all. The
compartment could be derived from the vault, but a harness should fail on missing
configuration rather than infer it. The encryption key is deliberately not named
ORACLE_KEY_OCID: ORACLE_KEY is already the PEM signing key, and two unrelated things one
suffix apart is the trap that produced this issue.

Build the OCI client in BeforeEach rather than the constructor. Ginkgo builds the whole
spec tree before applying a label filter, so the constructor runs on every provider's
leg, where these variables are deliberately empty, and a failure there would break
unrelated legs.

Two behaviours found by running it against a live tenancy rather than by reading. Vault
state transitions are asynchronous, so scheduling deletion against a secret still in
CREATING returns 409 IncorrectState, which is exactly when AfterEach calls it; delete now
waits for ACTIVE. And timeOfDeletion defaults to 30 days with an enforced 24 hour floor,
while a pending secret holds its quota slot, so deletion is scheduled at the minimum.
Also set a retry policy, since the SDK does not retry by default and five parallel specs
are enough for the service to answer 429.

Verified against a live OCI tenancy: 12 of 12 specs pass. The leg stays disabled; see
e2e/README.md for why a free tenancy cannot sustain it in CI.

Fixes: external-secrets/external-secrets#6767

Signed-off-by: Alexander Chernov <alexander@chernov.it>
Co-authored-by: Jean-Philippe Evrard <jean-philippe.evrard+rochepub@external.roche.com>

.github/workflows/e2e-reusable.yml

@@ -85,6 +85,12 @@ on:
         required: false
       ORACLE_KEY:
         required: false
+      ORACLE_VAULT_OCID:
+        required: false
+      ORACLE_COMPARTMENT_OCID:
+        required: false
+      ORACLE_ENCRYPTION_KEY_OCID:
+        required: false
 
 permissions:
   contents: read
@@ -311,4 +317,7 @@ jobs:
           ORACLE_REGION: ${{ contains(matrix.secret_groups, 'oracle') && secrets.ORACLE_REGION || '' }}
           ORACLE_FINGERPRINT: ${{ contains(matrix.secret_groups, 'oracle') && secrets.ORACLE_FINGERPRINT || '' }}
           ORACLE_KEY: ${{ contains(matrix.secret_groups, 'oracle') && secrets.ORACLE_KEY || '' }}
+          ORACLE_VAULT_OCID: ${{ contains(matrix.secret_groups, 'oracle') && secrets.ORACLE_VAULT_OCID || '' }}
+          ORACLE_COMPARTMENT_OCID: ${{ contains(matrix.secret_groups, 'oracle') && secrets.ORACLE_COMPARTMENT_OCID || '' }}
+          ORACLE_ENCRYPTION_KEY_OCID: ${{ contains(matrix.secret_groups, 'oracle') && secrets.ORACLE_ENCRYPTION_KEY_OCID || '' }}
         run: make -C e2e test.run

.github/workflows/e2e.yml

@@ -70,6 +70,9 @@ jobs:
       ORACLE_REGION: ${{ secrets.ORACLE_REGION }}
       ORACLE_FINGERPRINT: ${{ secrets.ORACLE_FINGERPRINT }}
       ORACLE_KEY: ${{ secrets.ORACLE_KEY }}
+      ORACLE_VAULT_OCID: ${{ secrets.ORACLE_VAULT_OCID }}
+      ORACLE_COMPARTMENT_OCID: ${{ secrets.ORACLE_COMPARTMENT_OCID }}
+      ORACLE_ENCRYPTION_KEY_OCID: ${{ secrets.ORACLE_ENCRYPTION_KEY_OCID }}
 
   # A /ok-to-test *comment* must carry an explicit sha=. A bare comment has no
   # trustworthy pinned commit (the dispatch would otherwise read the live PR
@@ -178,6 +181,9 @@ jobs:
       ORACLE_REGION: ${{ secrets.ORACLE_REGION }}
       ORACLE_FINGERPRINT: ${{ secrets.ORACLE_FINGERPRINT }}
       ORACLE_KEY: ${{ secrets.ORACLE_KEY }}
+      ORACLE_VAULT_OCID: ${{ secrets.ORACLE_VAULT_OCID }}
+      ORACLE_COMPARTMENT_OCID: ${{ secrets.ORACLE_COMPARTMENT_OCID }}
+      ORACLE_ENCRYPTION_KEY_OCID: ${{ secrets.ORACLE_ENCRYPTION_KEY_OCID }}
 
   # Report the fork run result back onto the originating pull request.
   report-fork:

e2e/README.md

@@ -180,3 +180,32 @@ make -C e2e test.run TEST_SUITES=provider GINKGO_LABELS="vault && !managed" \
 build if a provider is compiled into the suite but not covered by an area, if
 `needs_secrets` disagrees with `secret_groups`, or if an area names a secret
 group that the workflow does not wire.
+
+## Oracle: local only, by design
+
+The oracle suite runs against a real OCI tenancy and takes eight variables:
+`ORACLE_TENANCY_OCID`, `ORACLE_USER_OCID`, `ORACLE_REGION`,
+`ORACLE_FINGERPRINT`, `ORACLE_KEY` (the PEM signing key itself, which must have
+no passphrase), plus `ORACLE_VAULT_OCID`, `ORACLE_COMPARTMENT_OCID` and
+`ORACLE_ENCRYPTION_KEY_OCID`. The last three identify where secrets are created;
+the Vault API requires all of them and the `SecretStore` requires the vault.
+
+Note that `ORACLE_KEY` and `ORACLE_ENCRYPTION_KEY_OCID` are unrelated. The first
+is an RSA key that authenticates the caller; the second is the OCID of a
+symmetric AES key inside the vault that encrypts secrets at rest.
+
+**This leg is not enabled in CI, and that is a decision rather than a gap.** An
+always-free OCI tenancy caps secrets at 150, and a deleted secret cannot return
+to that pool for 24 hours; the floor is enforced, so values below it are
+rejected outright. One run of this suite creates 15 secrets, which allows about
+ten runs a rolling day before creates start failing for reasons unrelated to the
+change under test. `--flake-attempts=2` and `-p -nodes=5` in `entrypoint.sh`
+lower that further, the latter also being enough concurrency for the Vaults
+service to return `429`. Enabling this leg needs a tenancy without that cap.
+
+Until then the suite is a pre-submit tool for contributors with their own
+tenancy:
+
+```bash
+make -C e2e test.run TEST_SUITES=provider GINKGO_LABELS="oracle && !managed"
+```

e2e/run.sh

@@ -68,6 +68,9 @@ kubectl run --rm \
   --env="ORACLE_REGION=${ORACLE_REGION:-}" \
   --env="ORACLE_FINGERPRINT=${ORACLE_FINGERPRINT:-}" \
   --env="ORACLE_KEY=${ORACLE_KEY:-}" \
+  --env="ORACLE_VAULT_OCID=${ORACLE_VAULT_OCID:-}" \
+  --env="ORACLE_COMPARTMENT_OCID=${ORACLE_COMPARTMENT_OCID:-}" \
+  --env="ORACLE_ENCRYPTION_KEY_OCID=${ORACLE_ENCRYPTION_KEY_OCID:-}" \
   --env="SCALEWAY_API_URL=${SCALEWAY_API_URL:-}" \
   --env="SCALEWAY_REGION=${SCALEWAY_REGION:-}" \
   --env="SCALEWAY_PROJECT_ID=${SCALEWAY_PROJECT_ID:-}" \

e2e/suites/provider/cases/oracle/provider.go

@@ -16,96 +16,109 @@ limitations under the License.
 package oracle
 
 import (
-	"context"
+	"encoding/base64"
 	"os"
+	"sync"
+	"time"
 
-	// nolint
+	//nolint
 	. "github.com/onsi/ginkgo/v2"
 
-	// nolint
+	//nolint
 	. "github.com/onsi/gomega"
 	"github.com/oracle/oci-go-sdk/v65/common"
-	vault "github.com/oracle/oci-go-sdk/v65/vault"
+	"github.com/oracle/oci-go-sdk/v65/vault"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	utilpointer "k8s.io/utils/pointer"
 
 	"github.com/external-secrets/external-secrets-e2e/framework"
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 	esmeta "github.com/external-secrets/external-secrets/apis/meta/v1"
 )
 
-type oracleProvider struct {
-	tenancy     string
-	user        string
-	region      string
-	fingerprint string
-	privateKey  string
-	framework   *framework.Framework
-	ctx         context.Context
-}
-
 const (
-	secretName = "secretName"
+	// credentialsSecret is the Kubernetes Secret the SecretStore reads the API
+	// signing credentials from. Created in BeforeEach.
+	credentialsSecret = "provider-secret"
+
+	// retention is how far out a deleted secret is scheduled for removal. The
+	// Vault service accepts 1 to 30 days and enforces the floor, so a
+	// throwaway secret occupies a quota slot for a day whatever we do here.
+	// The 30 day default would hold it for a month, which an always-free
+	// tenancy (150 secrets) cannot absorb: one run of this suite creates 15.
+	retention = 25 * time.Hour
 )
 
-func newOracleProvider(f *framework.Framework, tenancy, user, region, fingerprint, privateKey string) *oracleProvider {
+type oracleProvider struct {
+	tenancy       string
+	user          string
+	region        string
+	fingerprint   string
+	privateKey    string
+	vaultID       string
+	compartment   string
+	encryptionKey string
+	framework     *framework.Framework
+
+	client vault.VaultsClient
+
+	// secretIDs maps the name a secret was created under to the OCID the
+	// service assigned it. Deletion addresses a secret by OCID, while the
+	// framework only ever knows the name it asked for.
+	mu        sync.Mutex
+	secretIDs map[string]string
+}
+
+func newFromEnv(f *framework.Framework) *oracleProvider {
 	prov := &oracleProvider{
-		tenancy:     tenancy,
-		user:        user,
-		region:      region,
-		fingerprint: fingerprint,
-		privateKey:  privateKey,
-		framework:   f,
+		tenancy:       os.Getenv("ORACLE_TENANCY_OCID"),
+		user:          os.Getenv("ORACLE_USER_OCID"),
+		region:        os.Getenv("ORACLE_REGION"),
+		fingerprint:   os.Getenv("ORACLE_FINGERPRINT"),
+		privateKey:    os.Getenv("ORACLE_KEY"),
+		vaultID:       os.Getenv("ORACLE_VAULT_OCID"),
+		compartment:   os.Getenv("ORACLE_COMPARTMENT_OCID"),
+		encryptionKey: os.Getenv("ORACLE_ENCRYPTION_KEY_OCID"),
+		framework:     f,
+		secretIDs:     map[string]string{},
 	}
 	BeforeEach(prov.BeforeEach)
 	return prov
 }
 
-func newFromEnv(f *framework.Framework) *oracleProvider {
-	tenancy := os.Getenv("OCI_TENANCY_OCID")
-	user := os.Getenv("OCI_USER_OCID")
-	region := os.Getenv("OCI_REGION")
-	fingerprint := os.Getenv("OCI_FINGERPRINT")
-	privateKey := os.Getenv("OCI_PRIVATE_KEY")
-	return newOracleProvider(f, tenancy, user, region, fingerprint, privateKey)
-}
-
-func (p *oracleProvider) CreateSecret(key string, val framework.SecretEntry) {
-	configurationProvider := common.NewRawConfigurationProvider(p.tenancy, p.user, p.region, p.fingerprint, p.privateKey, nil)
-	client, err := vault.NewVaultsClientWithConfigurationProvider(configurationProvider)
-	Expect(err).ToNot(HaveOccurred())
-	vmssecretrequest := vault.CreateSecretRequest{}
-	vmssecretrequest.SecretName = utilpointer.StringPtr(secretName)
-	vmssecretrequest.SecretContent = vault.Base64SecretContentDetails{
-		Name:    utilpointer.StringPtr(key),
-		Content: utilpointer.StringPtr(val.Value),
-	}
-	_, err = client.CreateSecret(p.ctx, vmssecretrequest)
+func (p *oracleProvider) BeforeEach() {
+	// The OCI client is built here rather than in the constructor. Ginkgo
+	// builds the whole spec tree before applying a label filter, so the
+	// constructor runs even on a leg testing some other provider, where these
+	// variables are deliberately empty. Failing there would break every other
+	// provider's run.
+	client, err := vault.NewVaultsClientWithConfigurationProvider(
+		// A nil passphrase means the API signing key must be unencrypted.
+		common.NewRawConfigurationProvider(
+			p.tenancy, p.user, p.region, p.fingerprint, p.privateKey, nil),
+	)
 	Expect(err).ToNot(HaveOccurred())
-}
 
-func (p *oracleProvider) DeleteSecret(key string) {
-	configurationProvider := common.NewRawConfigurationProvider(p.tenancy, p.user, p.region, p.fingerprint, p.privateKey, nil)
-	client, err := vault.NewVaultsClientWithConfigurationProvider(configurationProvider)
-	Expect(err).ToNot(HaveOccurred())
-	vmssecretrequest := vault.ScheduleSecretDeletionRequest{}
-	vmssecretrequest.SecretId = utilpointer.StringPtr(key)
-	_, err = client.ScheduleSecretDeletion(p.ctx, vmssecretrequest)
-	Expect(err).ToNot(HaveOccurred())
-}
+	// The SDK does not retry by default. Specs run in parallel and each one
+	// creates and deletes, which is enough concurrent traffic for the Vaults
+	// service to answer 429 TooManyRequests.
+	policy := common.DefaultRetryPolicy()
+	client.SetCustomClientConfiguration(common.CustomClientConfiguration{
+		RetryPolicy: &policy,
+	})
+	p.client = client
 
-func (p *oracleProvider) BeforeEach() {
-	OracleCreds := &v1.Secret{
+	oracleCreds := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      secretName,
+			Name:      credentialsSecret,
 			Namespace: p.framework.Namespace.Name,
 		},
 		StringData: map[string]string{
-			secretName: "value",
+			"keyid":     p.fingerprint,
+			"accesskey": p.privateKey,
 		},
 	}
-	err := p.framework.CRClient.Create(GinkgoT().Context(), OracleCreds)
+	err = p.framework.CRClient.Create(GinkgoT().Context(), oracleCreds)
 	Expect(err).ToNot(HaveOccurred())
 
 	secretStore := &esv1.SecretStore{
@@ -117,17 +130,17 @@ func (p *oracleProvider) BeforeEach() {
 			Provider: &esv1.SecretStoreProvider{
 				Oracle: &esv1.OracleProvider{
 					Region: p.region,
-					Vault:  "vaultOCID",
+					Vault:  p.vaultID,
 					Auth: &esv1.OracleAuth{
 						Tenancy: p.tenancy,
 						User:    p.user,
 						SecretRef: esv1.OracleSecretRef{
 							Fingerprint: esmeta.SecretKeySelector{
-								Name: "vms-secret",
+								Name: credentialsSecret,
 								Key:  "keyid",
 							},
 							PrivateKey: esmeta.SecretKeySelector{
-								Name: "vms-secret",
+								Name: credentialsSecret,
 								Key:  "accesskey",
 							},
 						},
@@ -139,3 +152,68 @@ func (p *oracleProvider) BeforeEach() {
 	err = p.framework.CRClient.Create(GinkgoT().Context(), secretStore)
 	Expect(err).ToNot(HaveOccurred())
 }
+
+func (p *oracleProvider) CreateSecret(key string, val framework.SecretEntry) {
+	// The secret's name is what the provider resolves remoteRef.key against,
+	// via GetSecretBundleByName, so it has to be the key the framework asked
+	// for. SecretContent.Name is only a label on the version.
+	content := base64.StdEncoding.EncodeToString([]byte(val.Value))
+	resp, err := p.client.CreateSecret(GinkgoT().Context(), vault.CreateSecretRequest{
+		CreateSecretDetails: vault.CreateSecretDetails{
+			CompartmentId: &p.compartment,
+			VaultId:       &p.vaultID,
+			KeyId:         &p.encryptionKey,
+			SecretName:    &key,
+			SecretContent: vault.Base64SecretContentDetails{
+				Name: &key,
+				// Documented as base64; the raw value would be stored
+				// mangled and fail to decode on read.
+				Content: &content,
+			},
+		},
+	})
+	Expect(err).ToNot(HaveOccurred())
+
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	p.secretIDs[key] = *resp.Id
+}
+
+func (p *oracleProvider) DeleteSecret(key string) {
+	p.mu.Lock()
+	id, ok := p.secretIDs[key]
+	delete(p.secretIDs, key)
+	p.mu.Unlock()
+	if !ok {
+		// CreateSecret never recorded an OCID for this key, so it failed and
+		// has already reported why. Staying quiet here keeps that first
+		// failure as the one the report shows.
+		return
+	}
+
+	// Vault state transitions are asynchronous. A secret is CREATING for a
+	// few seconds after the create call returns, and scheduling deletion in
+	// that window fails with 409 IncorrectState.
+	p.waitActive(id)
+
+	when := common.SDKTime{Time: time.Now().Add(retention)}
+	_, err := p.client.ScheduleSecretDeletion(GinkgoT().Context(), vault.ScheduleSecretDeletionRequest{
+		SecretId: &id,
+		ScheduleSecretDeletionDetails: vault.ScheduleSecretDeletionDetails{
+			TimeOfDeletion: &when,
+		},
+	})
+	Expect(err).ToNot(HaveOccurred())
+}
+
+func (p *oracleProvider) waitActive(id string) {
+	Eventually(func() (vault.SecretLifecycleStateEnum, error) {
+		resp, err := p.client.GetSecret(GinkgoT().Context(), vault.GetSecretRequest{
+			SecretId: &id,
+		})
+		if err != nil {
+			return "", err
+		}
+		return resp.LifecycleState, nil
+	}, time.Minute, 2*time.Second).Should(Equal(vault.SecretLifecycleStateActive))
+}